Strip comments where every name arrives, not at one source #24

Merged
branderson merged 1 commits from strip-comments-once into main 2026-08-31 23:31:15 -07:00
Collaborator

Summary generated by LLM agent

Follows PR 23, which fixed half of this. Reported from the same host, on the run straight after that merge.

The other source

PR 23 stripped comments from base-domains and stopped there. fetch_repo cats a fetched body raw, so a comment in a repository's own .dejima/allowed-domains reached valid_domain exactly as before:

dejima-ci-allowlist: refusing a malformed entry: #

A bare # is not an odd thing to write. It is the separator line in a comment block, which is how the generated files in these repositories are written, so the first project to document its own declaration was going to hit this whatever else the file contained.

Comments are stripped where every name arrives now, after collection rather than at each source. Both sources reach that point and so will any added later, which is the property the first fix lacked: it corrected the source I happened to be looking at.

The counts were of lines

grep -c . on the fetched body, so a comment block counted as declared names. A repository reporting 15 name(s) declared was declaring rather fewer, and the number was least accurate for the repositories that had bothered to explain themselves.

Not cosmetic given why that line exists: it was added in PR 23 so an operator could see what each repository contributed, and a count inflated by comments is a worse answer than none.

Also visible in the report

dejima/dejima-guard: no .dejima/allowed-domains on main is accurate rather than a fault. That project has no declaration file. Worth knowing if you expected one.

Tested: 151 passing, up from 148. Three new tests, all run against main and failing on it, plus an end-to-end run against a stubbed forge with comments in both the base file and the fetched body.

**Summary generated by LLM agent** Follows PR 23, which fixed half of this. Reported from the same host, on the run straight after that merge. ## The other source PR 23 stripped comments from `base-domains` and stopped there. `fetch_repo` `cat`s a fetched body raw, so a comment in a repository's own `.dejima/allowed-domains` reached `valid_domain` exactly as before: ``` dejima-ci-allowlist: refusing a malformed entry: # ``` A bare `#` is not an odd thing to write. It is the separator line in a comment block, which is how the generated files in these repositories are written, so the first project to document its own declaration was going to hit this whatever else the file contained. Comments are stripped where every name arrives now, after collection rather than at each source. Both sources reach that point and so will any added later, which is the property the first fix lacked: it corrected the source I happened to be looking at. ## The counts were of lines `grep -c .` on the fetched body, so a comment block counted as declared names. A repository reporting `15 name(s) declared` was declaring rather fewer, and the number was least accurate for the repositories that had bothered to explain themselves. Not cosmetic given why that line exists: it was added in PR 23 so an operator could see what each repository contributed, and a count inflated by comments is a worse answer than none. ## Also visible in the report `dejima/dejima-guard: no .dejima/allowed-domains on main` is accurate rather than a fault. That project has no declaration file. Worth knowing if you expected one. Tested: 151 passing, up from 148. Three new tests, all run against `main` and failing on it, plus an end-to-end run against a stubbed forge with comments in both the base file and the fetched body.
dejima-agent added 1 commit 2026-08-31 23:22:56 -07:00
Strip comments where every name arrives, not at one source
tests / runner-env (confined) (pull_request_target) Successful in 1m1s
tests / tests (pull_request_target) Successful in 1m20s
tests / runner-env (privileged) (pull_request_target) Successful in 38s
tests / compose-schema (pull_request_target) Successful in 46s
c539c82796
The previous commit stripped base-domains and stopped there. The next run on a
real host died the same way, on a bare `#` from a repository's own declaration:
a fetched body was cat'd raw too, so the bug was the same one a layer over, and
fixing the source I happened to be looking at left the other one open.

There is one place every name arrives, and comments are stripped there now.
Both sources reach it, and so will any source added later. A domain cannot
contain `#`, so nothing legitimate is lost.

The per-repository count was of lines, so a comment block counted as declared
names: "15 name(s) declared" for a file declaring rather fewer. It counts names.

The bare `#` is not an odd thing to write. It is the separator line in a
comment block, which is how the generated files in these repositories are
written, so the first project to document its own declaration would have hit
this whatever else it contained.
branderson approved these changes 2026-08-31 23:31:10 -07:00
branderson merged commit 3489b5f167 into main 2026-08-31 23:31:15 -07:00
Sign in to join this conversation.