Skip to content

chore(contributing): guard the duplicated CI-gate paragraph and ignore .devcontainer - #663

Merged
cevheri merged 3 commits into
libredb:mainfrom
7487:fix/661-dockerignore-devcontainer-and-ci-gate-guard
Sep 8, 2026
Merged

chore(contributing): guard the duplicated CI-gate paragraph and ignore .devcontainer#663
cevheri merged 3 commits into
libredb:mainfrom
7487:fix/661-dockerignore-devcontainer-and-ci-gate-guard

Conversation

@7487

@7487 7487 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Description

Both follow-ups from #658, as spec'd in #661.

Part 1 — .dockerignore. .devcontainer/ landed in #658 and was named in none of the groups, so the production build context carried it while every other non-input dot-directory was listed. Added under # Development files, beside .vscode and .idea. No test is owed here: .dockerignore has no executable lines for scripts/check-coverage.mjs to measure, and the repository has no guard over the file today.

Part 2 — the duplicated CI-gate paragraph. CONTRIBUTING.md step 4 and .github/curated-issue-footer.md carry the same **CI is the merge gate.** paragraph on purpose, and today they say the same words. Nothing kept them that way. tests/unit/curated-issue-footer.test.ts now reads the paragraph out of the footer at runtime and asserts CONTRIBUTING.md still repeats it. It deliberately holds no copy of its own — a literal in the test would be the third copy, drifting with the other two while staying green.

The comparison is on wording, not layout (this is the change from the first review round). The CONTRIBUTING.md copy is indented three spaces inside numbered list item 4 and the footer copy is flush left; the two also have different natural margins while this repository writes one sentence per line, so one of them will be re-flowed eventually without a word changing. Collapsing every run of whitespace on both sides covers indentation, re-wrapping and a stray trailing space in one step, and the reason is in a comment on the helper. A vacuity guard fails loudly if the footer stops containing a paragraph that opens with the marker, so the comparison can never pass against nothing.

CONTRIBUTING.md also gains a one-line pointer to .github/curated-issue-footer.md under Contributor programs — nothing in the tree referenced that file, so a maintainer wanting the footer had to retype it, which is what #618 was trying to stop. The third test pins that pointer.

Type of Change

  • Documentation update
  • Test addition or update

Related Issue

Closes #661

Changes Made

  • .dockerignore: .devcontainer under # Development files.
  • tests/unit/curated-issue-footer.test.ts: new drift guard, expected text derived from the footer, compared on wording rather than line layout.
  • CONTRIBUTING.md: pointer to .github/curated-issue-footer.md in the Contributor programs list.

Testing

The two red runs the issue asks for, both against tests/unit/curated-issue-footer.test.ts, re-run on b5faead after the review change:

  1. Paragraph deleted from CONTRIBUTING.md2 pass, 1 fail:
error: expect(received).toContain(expected)
Expected to contain: "**CI is the merge gate.** If you cannot run a command locally, list that command and the reason under a `Testing` heading in your PR body; submit the PR, and a maintainer will approve the fork's workflow run so CI can verify it. You do not need to withdraw correct work because a local tool is unavailable."
(fail) the CI-gate paragraph shared by CONTRIBUTING.md and the curated issue footer > CONTRIBUTING.md repeats it word for word
  1. One word changed in the footer copy (approve -> authorise) — 2 pass, 1 fail, and note the expected string now carries the new word, which is the proof it is read from the footer rather than from a literal:
error: expect(received).toContain(expected)
Expected to contain: "**CI is the merge gate.** If you cannot run a command locally, list that command and the reason under a `Testing` heading in your PR body; submit the PR, and a maintainer will authorise the fork's workflow run so CI can verify it. You do not need to withdraw correct work because a local tool is unavailable."
(fail) the CI-gate paragraph shared by CONTRIBUTING.md and the curated issue footer > CONTRIBUTING.md repeats it word for word

Both files restored, 3 pass, 0 fail.

All seven mutations from the review, same commit — the five real ones stay red, the two false positives are gone:

# Mutation Result
1 paragraph deleted from CONTRIBUTING.md red (2 pass, 1 fail)
2 one word changed in the footer red (2 pass, 1 fail), expected string carries the new word
3 one word changed in CONTRIBUTING.md red (2 pass, 1 fail)
4 marker removed from the footer named throw, 1 pass, 2 fail: .github/curated-issue-footer.md no longer contains a paragraph starting with **CI is the merge gate.**
5 pointer link removed from CONTRIBUTING.md red (2 pass, 1 fail), third test
6 CONTRIBUTING.md copy re-wrapped 4 lines to 5, no word changed green (was red)
7 single trailing space on one CONTRIBUTING.md line green (was red)

Gate, run locally (bun run test, never bare bun test):

  • bun run format — clean
  • bun run lint — 0 errors (135 pre-existing warnings, unchanged)
  • bun run typecheck — clean
  • bun run knip — clean (2 pre-existing config hints)
  • bun run readme:check, bun run chart:check, bun run channels:showcase:check, bun run security:check — all OK
  • bun run test10827 pass / 171 fail across 336 files. Baseline on upstream/main with these changes stashed: 10824 pass / 171 fail across 335 files. Exactly +3 pass, +1 file, identical failure set.

Commands I could not run locally, for CI to verify: the 171 failures above are entirely the helm-chart-* and packaging suites — helm and 7z are not installable in this sandbox, and CONTRIBUTING.md already documents that gap. For the same reason bun run test:coverage && bun run coverage:check cannot produce a merged lcov here; the change adds no executable lines under src/, and scripts/merge-lcov.mjs filters tests/ records out of coverage, so the 100% line gate is untouched either way. bun run build was also not run — nothing under src/ changed and next build does not read .dockerignore.

Test Environment

  • LibreDB Studio Version: 0.14.1 (main @ efcc0c2)
  • OS: macOS (darwin 25.6.0)
  • Node.js/Bun Version: Node 24.13.0, Bun 1.3.11

Checklist

  • My code follows the project's code style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

…e .devcontainer

.devcontainer/ arrived in libredb#658 and was named in none of the .dockerignore
groups, so the production build context carried it while every other non-input
dot-directory was listed. Added under "# Development files" beside .vscode and
.idea; the payload is tiny, the point is that the file stays readable.

The CI-gate paragraph lives in both CONTRIBUTING.md step 4 and
.github/curated-issue-footer.md on purpose, and nothing held the copies
together. tests/unit/curated-issue-footer.test.ts reads the paragraph out of the
footer at runtime - not from a literal, which would just be a third copy that
drifts while staying green - and asserts CONTRIBUTING.md still repeats it.
Leading whitespace is stripped on both sides because the CONTRIBUTING.md copy is
indented three spaces inside numbered list item 4.

CONTRIBUTING.md now also points at the footer file, which nothing in the tree
referenced, so a maintainer who wants it has a path other than retyping it.

Closes libredb#661

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cevheri cevheri added documentation Improvements or additions to documentation enhancement New feature or request deployment PaaS / one-click deployment platform integrations and listings dependencies Dependency version updates docker Container base image dependencies labels Sep 8, 2026

@cevheri cevheri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by mutation rather than by reading, and the important half is right: the expected wording
is read out of the footer at runtime, and I can prove your test does that rather than hold a copy,
because when I changed a word in the footer the failure message carried the new word. That was the
trap #661 was most worried about and you avoided it.

Five mutations behave exactly as they should. Paragraph deleted from CONTRIBUTING.md: red. One word
changed in the footer: red. One word changed in CONTRIBUTING.md (the direction you did not report,
so I ran it): red. Marker removed from the footer: your guard throws a named error instead of
passing on nothing. Pointer link removed: red. Your reasoning about the coverage gate is also
correct, the file is under tests/ and merge-lcov.mjs filters those records, so the throw line that
never runs cannot break the 100% gate.

Two mutations go red when they should not, both because the comparison is line-exact.

  1. Re-wrapping the paragraph in CONTRIBUTING.md with every word unchanged, 4 lines to 5: red.
    The two copies have different natural margins, the footer flush left and the CONTRIBUTING copy
    three spaces deep inside list item 4, and this repo's style asks for one sentence per line, so
    somebody will re-flow one of them without touching a word.
  2. A single trailing space at the end of one line: red, and the failure prints two strings that
    look identical, which is a bad half hour for whoever hits it. trimStart() leaves it.

Both disappear if the comparison is on wording rather than layout, which is also what "verbatim"
in the issue meant: line breaks are layout. I ran all seven mutations against this and the five
real ones stay red while the two false positives go green.

-    .map((line) => line.trimStart())
+    .map((line) => line.trim())
+
+const words = (text: string): string => text.replace(/\s+/g, " ").trim();

   test("CONTRIBUTING.md repeats it word for word", () => {
-    expect(dedent(read(CONTRIBUTING))).toContain(sharedParagraph());
+    expect(words(read(CONTRIBUTING))).toContain(words(sharedParagraph()));
   });

Keep your comment explaining the indentation, it is still the reason the naive comparison fails;
it just now covers wrapping too. Optional, take it or leave it: with the comparison independent of
layout, the first test asserting the paragraph spans more than one line is measuring a layout
property. Asserting it is longer than the marker itself would say the same thing without depending
on how it wraps.

Please re-run your two red proofs after the change and put them in the PR body as you did the
first time, so the guard is shown to still fail on real drift.

Verified separately: .devcontainer really is out of the image build context now. I built the
context and listed it, with .github absent as a control and src present as a positive one.
Static gates all green, and a full bun run test on your branch fails exactly the same set my
machine fails on main, so nothing new.

One correction that is mine, not yours. #661 said .dockerignore names every non-input
dot-directory. It does not quite: .cursorrules, .devin, .editorconfig, .nvmrc,
.oxlintrc.json and a few more are still in the context. My framing was stronger than the file
supports. You implemented what was asked, so nothing to change here.

Review on libredb#663: the line-exact comparison went red on two mutations that
change no word. Re-wrapping the CONTRIBUTING.md copy failed, and this
repository writes one sentence per line while the two copies have different
natural margins, so one of them gets re-flowed eventually. A single trailing
space also failed, printing two error strings that look identical.

Collapsing every run of whitespace covers indentation, wrapping and trailing
space together, so dedent() folds into the one normalizer instead of both
doing half the job. "Verbatim" in libredb#661 was about the words; line breaks are
layout.

The first test asserted the paragraph spans more than one line, which was the
same layout property. It now asserts the paragraph is longer than the marker
it is found by, which is what that test was really for.

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

7487 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Taken, both of them. b5faead.

You are right that line breaks are layout, and mutation 6 is the one that convinces me: the two
copies have different natural margins and the repo asks for one sentence per line, so a re-flow of
one of them is a matter of time, and a guard that goes red on it teaches people to distrust the
guard.

One deviation from your diff, and it is only about how many helpers do the job. Once words()
normalizes both sides for the comparison, dedent() no longer has a comparison to serve — including
the trimStart() -> trim() you suggested, since words() collapses that trailing space anyway. So
I folded the two into the one normalizer rather than keeping half a job in each: words() is now the
only whitespace helper, and sharedParagraph() trims each candidate paragraph before matching the
marker, which is all dedent() was still doing for it. Your comment survives on words(), extended
to cover wrapping and the trailing space as you asked. Behaviour is your diff's; I re-ran all seven
mutations against it and not just the two.

Optional half taken as well: the first test now asserts the paragraph is longer than the marker it
is found by, not that it spans more than one line.

Both red proofs are re-run and back in the PR body, plus the full seven-mutation table so the two
that flipped are visible next to the five that did not. Static gates green; bun run test unchanged
at 10827 pass / 171 fail across 336 files, same failure set as main.

One thing I did not touch, because it is your call rather than mine. On a genuine drift the failure
prints the whole of CONTRIBUTING.md as the received string, now on a single line — that was true
before this change too, but it is the remaining bad half hour. Reading the CONTRIBUTING.md
paragraph with the same finder and asserting the two paragraphs are equal would print just the two
of them, and would drop the whole-file read. It also moves mutation 1 from a comparison failure to
the named throw, which is why I have not done it unasked. Say the word and it is three lines.

And noted on #661's framing — no change needed here, but I would not have caught that myself.

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cevheri

cevheri commented Sep 8, 2026

Copy link
Copy Markdown
Member

Verified, and folding the two helpers into one is the better call. I re-ran your seven against b5faead plus four of my own: the five detections stay red, the re-wrap and the trailing space are green, and so is an indent-only change from three spaces to five. A dropped full stop, a reordered pair of sentences, and a footer paragraph cut back to the marker alone are all red. Mutation 2's message still carries the new word, so the wording is still derived at runtime. words() as the only normalizer does the whole job.

On the call you left me: do it. Two measurements decided it rather than taste.

The failure output goes from 20,510 characters to 1,510, and the two paragraphs land one above the other, so the changed word is visible instead of buried in the whole file printed on one line.

It is also more than cosmetic, which is the part neither of us had measured. Append a sentence to CONTRIBUTING.md's copy of the shared paragraph: the file still contains the footer's word sequence, so the guard stays green while the two documents say different things. Under equality that is red. You gain a sixth detection.

And the half you held back is an improvement, not a cost. CONTRIBUTING.md no longer contains a paragraph starting with **CI is the merge gate.** names the file and the reason. The thing it replaces is the 20 KB dump.

Here is the diff I measured, with format, lint, typecheck and knip green on it:

-/** The trimmed, blank-line-delimited paragraph of the footer that opens with `MARKER`. */
-const sharedParagraph = (): string => {
-  const found = read(FOOTER)
+/** The trimmed, blank-line-delimited paragraph of `relative` that opens with `MARKER`. */
+const paragraphIn = (relative: string): string => {
+  const found = read(relative)
     .split(/\n\s*\n/)
     .map((paragraph) => paragraph.trim())
     .find((paragraph) => paragraph.startsWith(MARKER));
   if (found === undefined) {
     // Rewording the footer's opening sentence is allowed; silently leaving this guard with nothing
     // to compare is not, because every assertion below would then pass on any CONTRIBUTING.md at all.
-    throw new Error(`${FOOTER} no longer contains a paragraph starting with ${MARKER}`);
+    throw new Error(`${relative} no longer contains a paragraph starting with ${MARKER}`);
   }
   return found;
 };
 
+const sharedParagraph = (): string => paragraphIn(FOOTER);
+
   test("CONTRIBUTING.md repeats it word for word", () => {
-    expect(words(read(CONTRIBUTING))).toContain(words(sharedParagraph()));
+    expect(words(paragraphIn(CONTRIBUTING))).toBe(words(sharedParagraph()));
   });

Push that and I will merge. The .dockerignore line and the pointer are unchanged and still verified, and the branch merges onto main cleanly with the guard green on the merged tree.

Reading CONTRIBUTING.md's copy with the same finder as the footer's makes the
assertion paragraph against paragraph. Containment against the whole file stayed
green when a sentence was appended to one copy, since the other's wording is
still present somewhere in the file, and on a real drift it printed the entire
file as the received string.
@cevheri

cevheri commented Sep 8, 2026

Copy link
Copy Markdown
Member

Pushed it myself as 091c748 rather than leave it waiting on a round trip, since this one is holding up other work. It is the diff from my last comment, plus two things I only noticed while making it:

  • The words() doc comment described the includes() comparison the previous commit had already replaced, so I reworded that sentence instead of leaving a comment that explains code which is no longer there.
  • The finder's throw now says "any pair of documents" rather than naming CONTRIBUTING.md, because it reads both sides.

I re-ran twelve mutations against it. Your seven behave as they did, my four extras behave (indent-only change green; dropped full stop, reordered sentences and a footer cut back to the marker all red), and the appended-sentence case is now red where it was green. Eight static gates green, and mutation 2's message still carries the new word, so the wording is still derived at runtime.

None of this is a correction of your work. Your commit is what made the comparison layout-independent; this only narrows what it compares to. If you would rather it read differently, push over it, it is your branch. Not merging until you have had a look.

@cevheri
cevheri merged commit 57b0e3f into libredb:main Sep 8, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Dependency version updates deployment PaaS / one-click deployment platform integrations and listings docker Container base image dependencies documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow-ups from #658: .dockerignore misses .devcontainer, and the CI-gate paragraph is duplicated with nothing measuring it

2 participants