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
64 changes: 63 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ ci/ everything that only exists to test/build the module
workflow_policy.py repo-policy checks over .github/workflows/
selftest.sh negative controls for the gate itself
fixtures/policy/ trees the policy checks must go RED on
PROMPT-standardize-module.md prompt: bring an existing module to this standard
PROMPT-standardize-module.md prompt: first-time bring-up of an existing
module (see also: syncing, below)
.githooks/pre-commit tracked commit gate (opt in: core.hooksPath)
.github/workflows/ the CI workflows, see below
```
Expand Down Expand Up @@ -274,6 +275,67 @@ vacuously.
is nginx-core noise only. A suppression over module frames turns the gate green
permanently.

## Syncing skeleton changes into a derived module

A module is cloned once and then drifts. This is the recurring half of that
relationship — the *first* bring-up of an existing module is a different, much
larger job and has its own document:
**[ci/PROMPT-standardize-module.md](ci/PROMPT-standardize-module.md)**.

Use that prompt when the target has never been standardised (no `ci/` layout, no
`ci.yml` orchestrator, no `ci/linter/`). Use the list below when the target is
already on the standard and you are forwarding a specific improvement.

**Per module, one session, one repo:**

- **Establish the anchor.** Whatever the target last took from here — a
`CHANGES` entry, a `vN` tag, or the merge commit of its standardisation PR.
Without an anchor you are guessing at scope. `git log --oneline <anchor>..HEAD`
in this repo is the candidate set; [CHANGES](CHANGES) says what each one was
*for*.
- **Select, don't sweep.** Take one concern per PR. A sync PR carrying four
unrelated skeleton commits is not independently revertible and its CI failure
does not tell you which change broke it.
- **Re-derive, never copy blind.** Every workflow here carries skeleton-specific
paths, file names, runner labels and version pins. A file copied verbatim
compiles, runs, and gates the wrong thing. The reference for what each gate
must *prove* is the Phase 2 table in the prompt.
- **Check the four drift classes first** — none is visible from a green run,
each has bitten a module here:
- a job that binds test ports without its own declared band. Presence of
`TEST_NGINX_PORT` is not the check — the check is a **distinct job-level
band**, verified free with `ci/tools/max-port.sh` before anything binds it.
Test::Nginx's default 1984 is unarbitrated and `builder02` runs six slots
against one network, so the collision reads as a module regression
(`bind() … Address already in use`). Note the reference itself currently
runs that verification before the *runtime* suite only, not before `prove`.
- `--gcov-object-directory` in `ci/tools/coverage.sh`. The condition is the
**gcovr major version the job actually runs**, not whether the pin exists:
the `--gcov-`-prefixed spelling arrived in gcovr 7.0 and fails argparse
below it. `--object-directory` is accepted by both and is the portable
choice for a target whose runner gcovr you do not control — the fork arm's
`ubuntu-latest`.
- a `.github/versions.env` consumer that sources the file without validating
it executes any line that is not a pin.
- `ci/linter/workflow_policy.py` older than the YAML-parse rewrite matches
workflow YAML with regexes, and valid YAML (`.yaml` extension, inline
`on: [pull_request]`, a comment after a job key) makes all three policy
checks silently vacuous.

The last two only exist in a module that already took those files; the first
two apply to any module that runs `prove` or reports coverage.
- **Verify the gate red.** A gate you moved but never saw fail is a gate you did
not verify. State the probe in the PR body.
- **Remote CI green before merge**, then squash, delete the branch, and bump the
superrepo gitlink for that checkout.
- **Send improvements back.** A fix or a layer the target grew that this repo
lacks gets a PR *here*. The template is only worth keeping if it stays ahead
of its clones.

Record the anchor you synced to in the target's memory mirror
(`/opt/myguard/memory/labs/<module-name>/index.md`), or the next session
re-derives it from scratch.

## Requirements

Build: `build-essential curl libpcre2-dev zlib1g-dev`
Expand Down
32 changes: 32 additions & 0 deletions ci/PROMPT-standardize-module.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,20 @@ The reference implementation is `/opt/myguard/labs/nginx-skeleton-module`
(this repo). **Read the reference before changing the target** — the point is
not to make the target look similar, it is to give it the same *gates*.

## Scope — is this the right document?

This is the **first-time** bring-up: a module that has never been standardised.
It is eight phases, eight PRs, and one fresh session per module.

If the target is **already** on this standard and you are forwarding one later
skeleton improvement into it, this document is the wrong size for the job. Use
the sync checklist in the reference's README instead — *Syncing skeleton changes
into a derived module* — and come back here only for a phase the target never
took at all.

Tell which you are looking at: a standardised target has a `ci/` layout, a
`ci.yml` orchestrator as its sole `pull_request` entry point, and `ci/linter/`.

---

## Context you are given
Expand Down Expand Up @@ -136,6 +150,24 @@ Also port, adapting paths:
Members are `workflow_call:`-only; only `ci.yml` has `pull_request:`. Two entry
points run everything twice per PR and defeat the laning.

**Every job that runs `prove` must pin its own port band.** Test::Nginx binds
`TEST_NGINX_PORT`, default 1984, and nothing arbitrates it; `builder02` runs six
slots against one network, so two jobs on the default collide and the loser dies
with `bind() to 127.0.0.1:1984 failed (98: Address already in use)` — which reads
as a module regression and is not one. The reference declares a distinct
job-level `TEST_BASE_PORT` per workflow (`build-test.yml` 19200, `ci-deep.yml`
19400) and passes it as `TEST_NGINX_PORT`; `ci/linter/lint-ci-ports.sh` fails the
build if a job starts the runtime driver without declaring a band. Give the
target its own band — two workflows sharing one number is the same bug with more
steps.

`ci/tools/max-port.sh` proves a band is free and clear of the kernel ephemeral
range, and **belongs before the first step that binds it**. In the reference it
currently sits after `prove` and before the runtime suite, so `ci/t/` binds
unverified; put it earlier in the target rather than copying that order. The
whole value of the check is turning "unreproducible timeout" into a named
failure, which it cannot do for a step that already ran.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
**Badges:** README badge block must list the workflows in the SAME ORDER as the
`## CI` table in that README, and both must match reality — a badge for a
workflow that no longer exists renders a permanent grey "no status" and is
Expand Down
Loading