ci: pin the review follow-ups of the CI workflow (#33) #34

Closed

maximus wants to merge 0 commits from issue-33-ci-review-followups into main

maximus commented

2026-09-28 00:55:01 +00:00

Owner

Fixes #33

Suite de la revue de la PR #31 : les trois constats non bloquants.

1. Aucun continue-on-error dans le workflow. Nouveau test dans __tests__/ci.test.js, ancré sur la position de clé (^\s*(- )?continue-on-error\s*:), sur le YAML débarrassé de ses lignes de commentaire.

2. Port déjà occupé. scripts/boot-from-copy.sh sonde le port avant de démarrer : libre seulement si curl répond « connexion refusée » (exit 7). Toute autre sortie (réponse HTTP, écoute muette, service non HTTP) fait échouer le script avec port N is already taken. --noproxy '*' empêche un http_proxy de répondre à la place du port. BOOT_PORT est documenté dans l'en-tête. Le test lance un vrai voisin HTTP sur 127.0.0.1 et le script en spawn asynchrone (un spawnSync bloquerait la boucle d'événements et le voisin ne répondrait jamais).

3. Modules chargés au démarrage. Pour chaque .js du COPY, chaque ligne require( doit commencer en colonne 0, et aucun import( n'est permis (commentaires ignorés). C'est une heuristique : elle rend exhaustif un boot check qui n'appelle que /health.

Preuve par mutation (fichiers restaurés depuis une copie, git diff --stat identique avant/après) :

Mutation	Résultat
`continue-on-error: true` sous l'étape `npm test`	1/12 rouge : `never lets a failing step pass`
`require("node:path")` déplacé dans une fonction de `index.js`	1/12 rouge : `index.js requires only at module level`
pré-contrôle du port neutralisé	1/12 rouge : le script interroge le voisin, `GET /health without token: expected 401`

La troisième mutation reproduit le défaut décrit par la revue : sans le pré-contrôle, les assertions portent sur un serveur que le script n'a pas lancé.

Suite locale : 313/313 vitest, 45/45 test-curl.sh, boot-from-copy: PASS.

Aucun fichier runtime touché (ni index.js, ni metrics.js, ni Dockerfile) : pas de redéploiement nécessaire.

🤖 Generated with Claude Code

Fixes #33 Suite de la revue de la PR #31 : les trois constats non bloquants. **1. Aucun `continue-on-error` dans le workflow.** Nouveau test dans `__tests__/ci.test.js`, ancré sur la position de clé (`^\s*(- )?continue-on-error\s*:`), sur le YAML débarrassé de ses lignes de commentaire. **2. Port déjà occupé.** `scripts/boot-from-copy.sh` sonde le port avant de démarrer : libre seulement si curl répond « connexion refusée » (exit 7). Toute autre sortie (réponse HTTP, écoute muette, service non HTTP) fait échouer le script avec `port N is already taken`. `--noproxy '*'` empêche un `http_proxy` de répondre à la place du port. `BOOT_PORT` est documenté dans l'en-tête. Le test lance un vrai voisin HTTP sur `127.0.0.1` et le script en `spawn` asynchrone (un `spawnSync` bloquerait la boucle d'événements et le voisin ne répondrait jamais). **3. Modules chargés au démarrage.** Pour chaque `.js` du `COPY`, chaque ligne `require(` doit commencer en colonne 0, et aucun `import(` n'est permis (commentaires ignorés). C'est une heuristique : elle rend exhaustif un boot check qui n'appelle que `/health`. **Preuve par mutation** (fichiers restaurés depuis une copie, `git diff --stat` identique avant/après) : | Mutation | Résultat | |---|---| | `continue-on-error: true` sous l'étape `npm test` | 1/12 rouge : `never lets a failing step pass` | | `require("node:path")` déplacé dans une fonction de `index.js` | 1/12 rouge : `index.js requires only at module level` | | pré-contrôle du port neutralisé | 1/12 rouge : le script interroge le voisin, `GET /health without token: expected 401` | La troisième mutation reproduit le défaut décrit par la revue : sans le pré-contrôle, les assertions portent sur un serveur que le script n'a pas lancé. **Suite locale** : 313/313 vitest, 45/45 `test-curl.sh`, `boot-from-copy: PASS`. Aucun fichier runtime touché (ni `index.js`, ni `metrics.js`, ni `Dockerfile`) : pas de redéploiement nécessaire. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

maximus added 1 commit 2026-09-28 00:55:02 +00:00

ci: pin the review follow-ups of the CI workflow (#33 )

CI / verification (pull_request) Successful in 19s

Details

CI / verification (push) Successful in 20s

Details

156ca818ca

- Forbid `continue-on-error` in ci.yml: that mutation left every test green,
  and a failing step would turn the commit status green in silence.
- boot-from-copy.sh refuses a port already held by something else (any curl
  exit other than 7). Without it, the wait loop could reach the neighbour and
  assert on a server it never started — the new test reproduces exactly that
  when the check is removed.
- Pin every require of the COPY'd runtime modules at column 0, with no
  dynamic import: the boot check only calls /health, so it only proves the
  modules loaded at startup.

Fixes #33

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

maximus commented

2026-09-29 23:35:22 +00:00

Author

Owner

Review — APPROVE

Résumé : les trois constats de la revue de #31 sont traités, et chacune des trois nouvelles épingles rougit sous la mutation qu'elle est censée interdire. Aucun fichier runtime touché. CI verte sur 156ca81 (CI / verification (pull_request) : success).

Vérification par mutation (faite par le relecteur, sur une copie hors dépôt, arbre restauré et comparé par `diff -r`)

Mutation	Résultat
`continue-on-error: true` sous l'étape `npm test`	1/12 rouge (`never lets a failing step pass`)
`require("./nope.js")` indenté dans une fonction de `index.js`	1/12 rouge (`index.js requires only at module level`)
pré-contrôle du port neutralisé (`true \|\| fail`)	1/12 rouge : le script interroge le voisin, `expected 401, got 200` — le défaut signalé par la revue est bien reproduit

Local : 313/313 vitest, boot-from-copy: PASS. Le commit porte Fixes #33.

Suggestions (non bloquantes)

--noproxy appliqué au seul pré-contrôle (scripts/boot-from-copy.sh). La boucle d'attente et les deux assertions appellent curl sans --noproxy. Vérifié avec curl 8.5 : http_proxy=http://127.0.0.1:9 sh scripts/boot-from-copy.sh → pré-contrôle « port libre », serveur démarré (listening on :3097), puis FAIL: the server did not answer on :3097 within 10 s. L'échec est bruyant, pas un faux vert, mais le raisonnement du commentaire (« un proxy peut répondre à la place du port ») vaut autant pour les assertions. Un export NO_PROXY='*' no_proxy='*' en tête de script couvrirait tous les appels d'un coup.
Trous connus de l'heuristique « require en colonne 0 » (assumée comme heuristique dans la PR, je les consigne pour mémoire) : restent verts function lazy() { return require("./x"); } sur une seule ligne en colonne 0, et un require( renvoyé en colonne 0 sous const x = à l'intérieur d'une fonction. Ni l'un ni l'autre n'est une forme naturelle sous le style du dépôt ; pas besoin de plus.
continue-on-error en style flow : - { name: x, run: false, continue-on-error: true } passe l'épingle (ancrée sur la position de clé). Forme improbable dans ce workflow ; un \bcontinue-on-error\s*: non ancré sur ciCode (déjà sans commentaires) fermerait le cas sans faux positif réaliste.

🤖 Generated with Claude Code

## Review — APPROVE **Résumé** : les trois constats de la revue de #31 sont traités, et chacune des trois nouvelles épingles rougit sous la mutation qu'elle est censée interdire. Aucun fichier runtime touché. CI verte sur `156ca81` (`CI / verification (pull_request)` : success). ### Vérification par mutation (faite par le relecteur, sur une copie hors dépôt, arbre restauré et comparé par `diff -r`) | Mutation | Résultat | |---|---| | `continue-on-error: true` sous l'étape `npm test` | 1/12 rouge (`never lets a failing step pass`) | | `require("./nope.js")` indenté dans une fonction de `index.js` | 1/12 rouge (`index.js requires only at module level`) | | pré-contrôle du port neutralisé (`true \|\| fail`) | 1/12 rouge : le script interroge le voisin, `expected 401, got 200` — le défaut signalé par la revue est bien reproduit | Local : 313/313 vitest, `boot-from-copy: PASS`. Le commit porte `Fixes #33`. ### Suggestions (non bloquantes) 1. **`--noproxy` appliqué au seul pré-contrôle** (`scripts/boot-from-copy.sh`). La boucle d'attente et les deux assertions appellent `curl` sans `--noproxy`. Vérifié avec curl 8.5 : `http_proxy=http://127.0.0.1:9 sh scripts/boot-from-copy.sh` → pré-contrôle « port libre », serveur démarré (`listening on :3097`), puis `FAIL: the server did not answer on :3097 within 10 s`. L'échec est bruyant, pas un faux vert, mais le raisonnement du commentaire (« un proxy peut répondre à la place du port ») vaut autant pour les assertions. Un `export NO_PROXY='*' no_proxy='*'` en tête de script couvrirait tous les appels d'un coup. 2. **Trous connus de l'heuristique « `require` en colonne 0 »** (assumée comme heuristique dans la PR, je les consigne pour mémoire) : restent verts `function lazy() { return require("./x"); }` sur une seule ligne en colonne 0, et un `require(` renvoyé en colonne 0 sous `const x =` à l'intérieur d'une fonction. Ni l'un ni l'autre n'est une forme naturelle sous le style du dépôt ; pas besoin de plus. 3. **`continue-on-error` en style flow** : `- { name: x, run: false, continue-on-error: true }` passe l'épingle (ancrée sur la position de clé). Forme improbable dans ce workflow ; un `\bcontinue-on-error\s*:` non ancré sur `ciCode` (déjà sans commentaires) fermerait le cas sans faux positif réaliste. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

maximus commented

2026-09-29 23:38:55 +00:00

Author

Owner

Mergé en fast-forward sur main (156ca81) après rejeu local du tip : npm ci, 313/313 vitest, 45/45 test-curl.sh, boot-from-copy: PASS. Forgejo ne détecte pas le ff local, fermeture manuelle. #33 fermée par le Fixes #33 du commit. Suggestions non bloquantes de la revue reportées dans une issue de suivi.

Mergé en fast-forward sur `main` (`156ca81`) après rejeu local du tip : `npm ci`, 313/313 vitest, 45/45 `test-curl.sh`, `boot-from-copy: PASS`. Forgejo ne détecte pas le ff local, fermeture manuelle. #33 fermée par le `Fixes #33` du commit. Suggestions non bloquantes de la revue reportées dans une issue de suivi.