Sponsors: footer link, README section and a daily sponsor-list sync - #82
Conversation
Deploying amnesia-site with
|
| Latest commit: |
28eea9a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://abfdbc61.amnesia-site.pages.dev |
| Branch Preview URL: | https://claude-sponsors-funnel.amnesia-site.pages.dev |
The page footers say "no ads, funded by sponsors" and link to GitHub Sponsors. The README gains a Sponsor section and badge; its block is rebuilt by scripts/sponsors.mjs from the public sponsor list, and sponsors-readme.yml runs it daily and opens a PR when it changes.
e292ab3 to
d085474
Compare
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Request changes: the sponsor sync silently omits eligible public sponsors after the first API page. rule:none
Blocking — correctness — scripts/sponsors.mjs:33-34
sponsorshipsAsMaintainer(first: 100, activeOnly: true, includePrivate: false) {
nodes {
The query asks for at most 100 sponsorship nodes and neither requests pageInfo nor follows an endCursor. If the maintainer has 101 or more active public sponsorships (in particular, 101 recurring sponsors at $25/month or above), an eligible sponsor outside this first page is never passed to normalizeSponsors or rendered into the README. That violates the newly introduced promise to list sponsors at that tier, while the workflow succeeds and reports the incomplete block as current.
sponsorshipsAsMaintainer(first: 100, after: $cursor, activeOnly: true, includePrivate: false) {
nodes { /* existing fields */ }
pageInfo { hasNextPage endCursor }
}
Fetch successive pages until hasNextPage is false, combine their nodes before normalization, and add a regression test covering a multi-page response.
I reviewed the workflow permissions and force-push/PR flow, README marker replacement, sponsor filtering, footer/CSP changes, and the added unit tests. Required CI checks are green at d085474c298bec78ba14ca8457b71cb7b72641b1; I did not run the local test suite.
The query read only the first 100 sponsorships. It now requests pageInfo, follows endCursor until hasNextPage is false, and fails if a cursor does not advance.
|
The failing |
sprayberry-secondread
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the Claude second-opinion lane (independent second read; the gating review is posted separately).
Verdict: the site and README changes are fine. There are two problems in the new sync. The bot's PR can never get its required checks, and the $25 cutoff has no test at its exact value.
Head reviewed: 963336e (live head). The ticket was cut at d085474. Since then c7a891f (pagination) and a merge of main have landed, so this read covers the full PR diff at 963336e.
1. Medium: the bot PR is opened with GITHUB_TOKEN, so its required checks never run
.github/workflows/sponsors-readme.yml:57
- name: Open or refresh the PR
if: steps.sync.outputs.changed == 'true'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}What goes wrong: a new public sponsor signs up at $25/month or more. The 06:53 run rewrites the block, force-pushes bot/sponsors-readme and opens the PR, all with GITHUB_TOKEN. GitHub does not start pull_request workflows for pushes or PRs made with GITHUB_TOKEN. That means ci.yml (Validate HTML and Scripts, Scorecard (file-based checks)) and codeql.yml (Analyze (actions), Analyze (javascript-typescript)) never report on that PR. The main ruleset requires all four (required_status_checks, strict). The PR sits at "Expected — Waiting for status to be reported" and can't merge unless a human closes and reopens it, pushes to it, or uses an admin bypass. Every later daily run force-pushes with the same token, so the problem comes back each time. The workflow header says it keeps the $25 tier's promise "without anyone remembering to", and this is where that breaks.
dario's version, which this was ported from, handles exactly this at the same step. The port dropped it:
# A PAT when one is configured, so the PR's checks run (GITHUB_TOKEN
# pushes do not trigger workflows); GITHUB_TOKEN otherwise.
GH_TOKEN: ${{ secrets.DARIO_DRIFT_BOT_PAT || secrets.GITHUB_TOKEN }}The checkout at line 30 uses persist-credentials: true with the default token, so the git push at line 68 also runs as GITHUB_TOKEN. Changing GH_TOKEN alone doesn't fix the push.
Suggested fix
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
# A PAT when one is configured, so the bot PR's required checks run
# (GITHUB_TOKEN pushes and PRs do not trigger workflows).
token: ${{ secrets.AMNESIA_BOT_PAT || secrets.GITHUB_TOKEN }}
persist-credentials: true
...
- name: Open or refresh the PR
env:
GH_TOKEN: ${{ secrets.AMNESIA_BOT_PAT || secrets.GITHUB_TOKEN }}If you don't want a PAT, write down the manual step (close and reopen the bot PR) in the workflow header and the README <sub> line. Right now both say it just happens.
2. Medium: the >= $25 cutoff has no test at its exact value
scripts/sponsors.mjs:65
const named = sponsors.filter((s) => !s.oneTime && s.monthly >= README_TIER_MIN_USD);test/sponsors.test.mjs:25-33 renders only 100 (named), 5 (not named) and a one-time 500. Two other tests use a $25 sponsor (ada at line 19, first at line 59), but both only check normalizeSponsors order and never call renderReadmeBlock. Change >= to > and all six tests still pass. A $25/month sponsor, the tier the promise is written for, would then be dropped from the README without any test failing.
Suggested fix
test('only recurring sponsors at $25/month and up are named', () => {
const block = renderReadmeBlock(normalizeSponsors([
node('big', 100, { sponsorEntity: { login: 'big', name: ' Big Co ' } }),
node('edge', 25),
node('under', 24),
node('small', 5),
node('once', 500, { isOneTimePayment: true }),
]));
assert.match(block, /- \[@big\]\(https:\/\/github\.com\/big\) \(Big Co\)/);
assert.match(block, /\[@edge\]/);
assert.doesNotMatch(block, /under|small|once/);
});3. Low: sponsor display names go into the README without escaping
scripts/sponsors.mjs:72
for (const s of named) lines.push(`- ${mention(s)}${s.name ? ` (${s.name})` : ''}`);name is whatever the sponsor puts in their GitHub profile. A name like x](https://example.net) [y renders as a link in the README. Every refresh goes through a PR review, so this doesn't block. Escaping []() in name would remove it.
Smaller notes (not blocking)
- The one-time filter only has a test for the
isOneTimePaymentside. A node where onlytier.isOneTimeis true isn't covered, so dropping then.tier && n.tier.isOneTimeclause atscripts/sponsors.mjs:55still passes. - The PR body's Verification section still says "4 of them new" in
test/sponsors.test.mjs.c7a891fadded two pagination tests, so there are now six.
Boundaries I rebuilt from the diff
| Row | Code at head | Pinned by |
|---|---|---|
monthly 24 / 5 |
not named | test 2 (small, 5) |
monthly == 25 |
named | none (finding 2) |
one-time via isOneTimePayment |
not named | test 2 (once) |
one-time via tier.isOneTime only |
not named | none (low) |
login '', null entity, null node, undefined list |
dropped / [] |
test 1 |
| zero named sponsors | one sentence, 3 lines | test 3 |
| markers missing | throws | test 4 |
hasNextPage with a cursor |
follows it | test 5 |
hasNextPage with null cursor |
throws, --write exits 1 |
test 6 |
HTTP not ok / GraphQL errors with data: null |
throws, exit 1 | none, but the code does the right thing (conn is undefined, so it throws) |
What's good
- Both footers are plain
<a>links with notarget, script or image.Referrer-Policyissame-originon Pages andno-referrerin the self-host image (image/amnesia_app.py:96), so the body's no-referrer claim holds on both. The CSP hash change insrc/_headerspassesValidate HTML and Scripts. --writefails closed: a failed read or a pagination cursor that doesn't advance exits 1 before the README is touched, so a bad read can't empty the block.- In the workflow, actions are pinned by SHA, top-level permissions are
contents: read, and there's a repository guard and a concurrency group.
Not read: the GraphQL call against a real token (the body says the same), and the generated README badge's rendering. CI at head is green on all four required checks. The failing kick check comes from the separate review-kick workflow.
SECOND READ: NOT READY — sponsors-readme.yml:57 opens the bot PR with GITHUB_TOKEN so its required checks never run, and the $25 cutoff (sponsors.mjs:65) has no test at 25
The sync pushed and opened its PR with GITHUB_TOKEN, which starts no workflows, so the required checks never reported. Checkout and gh now use AMNESIA_BOT_PAT when it is set; the header and README say what to do without it. The tier test now covers exactly $25 and $24.
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Request changes: unescaped sponsor display names let a sponsor inject active Markdown into the README. rule:none
Blocking — security/privacy — scripts/sponsors.mjs:72
for (const s of named) lines.push(- ${mention(s)}${s.name ?(${s.name}): ''});
name is supplied by each sponsor's GitHub profile and is inserted directly into Markdown. For example, a recurring $25 sponsor can set its display name to . The daily workflow then writes that into the public README; GitHub renders it as an external image when visitors view the README, creating an unreviewed third-party request despite this PR's new no-tracking promise. The existing test only exercises the benign name Big Co, so it does not pin that display names are rendered as text.
const escapeMarkdownText = (value) => value.replace(/[\\`*_[\]<>!()]/g, '\\$&');
for (const s of named) {
lines.push(`- ${mention(s)}${s.name ? ` (${escapeMarkdownText(s.name)})` : ''}`);
}
Add a test with Markdown image/link syntax in name and assert the generated block contains escaped text rather than a rendered Markdown construct.
What I checked: I reviewed the full live diff, including the rework that changes the checkout and gh credentials to use AMNESIA_BOT_PAT when available and adds the $25/$24 cutoff test. The previously reported pagination and bot-check blockers are fixed at this head. All four required CI checks pass at d6c7a3a006792bbad46d211da05974f7012df518. I did not run the local test suite or make a live GraphQL request.
sprayberry-secondread
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the Claude second-opinion lane (independent second read; the gating review is posted separately).
Verdict: ready. d6c7a3a fixes both things my read at 963336e held against it. Two low notes remain, and neither blocks.
Head reviewed: d6c7a3a. I read the full PR diff and the 963336e...d6c7a3a compare, which touches only .github/workflows/sponsors-readme.yml, README.md and test/sponsors.test.mjs.
What d6c7a3a fixes
- The bot PR gets its checks.
.github/workflows/sponsors-readme.yml:37andtoken: ${{ secrets.AMNESIA_BOT_PAT || secrets.GITHUB_TOKEN }}
.github/workflows/sponsors-readme.yml:63Checkout keepsGH_TOKEN: ${{ secrets.AMNESIA_BOT_PAT || secrets.GITHUB_TOKEN }}
persist-credentials: true, so thegit push --forceruns as the PAT, and so doesgh pr create. That covers both halves of the problem I raised: the old code changed onlyGH_TOKEN, which left the push onGITHUB_TOKEN. An unset secret evaluates to'', so||falls back toGITHUB_TOKEN. The header and the README<sub>line say what happens without the PAT (close and reopen the PR). That's accurate: a humanreopenedevent does startpull_requestworkflows. The sync step still reads GraphQL withGITHUB_TOKENonly, which is all it needs. - The $25 cutoff is pinned.
test/sponsors.test.mjs:28-29,34-35Ifnode('edge', 25), node('under', 24), ... assert.match(block, /\[@edge\]/); assert.doesNotMatch(block, /under|small|once/);
>=atscripts/sponsors.mjs:65becomes>, theedgeassertion fails. If the cutoff drops to 24,undergets named and thedoesNotMatchfails. Neither pattern can match the fixed text of the block ("funded", "users", "Thank you"), so both assertions can fail.
Low notes (not blocking)
- One-time filter,
tier.isOneTimeside.scripts/sponsors.mjs:55The only one-time test case setsconst oneTime = Boolean(n.isOneTimePayment || (n.tier && n.tier.isOneTime));
isOneTimePayment: true. You could delete then.tier && n.tier.isOneTimeclause and every test would still pass. The clause is a backup for a field that is normally set along withisOneTimePayment, which is why I'm not blocking on it. A one-line case pins it if you want it covered:node('tieronce', 500, { tier: { monthlyPriceInDollars: 500, isOneTime: true } }),
- The PR body's test count is out of date. Verification says "4 of them new in
test/sponsors.test.mjs". The file has six tests: the twofetchSponsorspagination tests were added after the body was written. The code does everything the body lists, so the count is only an understatement.
The unescaped display name at scripts/sponsors.mjs:72, which I noted at 963336e, is unchanged. It is still low: every refresh goes through a PR review.
Boundaries at this head
| Row | Code | Pinned by |
|---|---|---|
monthly 24 / 5 |
not named | test 2 (under, small) |
monthly == 25 |
named | test 2 (edge) |
one-time via isOneTimePayment |
not named | test 2 (once) |
one-time via tier.isOneTime only |
not named | none (low note 1) |
login '', null entity, null node, undefined list |
dropped / [] |
test 1 |
| zero named sponsors | one sentence, 3 lines | test 3 |
| markers missing | throws | test 4 |
hasNextPage with a cursor |
follows it | test 5 |
hasNextPage with a null cursor |
throws; --write exits 1 |
test 6 |
AMNESIA_BOT_PAT unset |
falls back to GITHUB_TOKEN, as documented |
workflow; no unit test applies |
What's good
--writefails closed. A failed read or a stuck cursor exits 1 before the README is touched.- The footers are plain
<a>links with no script, image ortarget. The regenerated CSP hash passesValidate HTML and Scripts. - Actions are pinned by SHA, top-level permissions are
contents: read, and the job has a repository guard and a concurrency group.
What I checked and didn't: CI at head passes on Validate HTML and Scripts, Scorecard (file-based checks), both CodeQL analyses and Build and smoke-test. The failing kick job is the review-kick dispatcher, which failed with "Interactive authentication required" on systemctl. That isn't this PR. I didn't exercise the live GraphQL read, and I can't see whether AMNESIA_BOT_PAT is set. I didn't run tests locally.
SECOND READ: READY
A sponsor sets their own display name, and it went into the README as Markdown: a name like  would have made every README view load a third-party image. Names are now escaped and whitespace is collapsed, with a test using image, HTML, link and newline syntax.
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Verdict: APPROVED — no blocking issues found in the live-head change.
I reviewed the sponsor-list GraphQL pagination, its non-advancing-cursor failure guard, recurring/$25 tier filtering, Markdown escaping for sponsor-controlled display names, README replacement markers, and the scheduled workflow's scoped permissions and bot-token fallback. Required CI is passing at 6e56cb5ad183cb010e9f93b8c3f0e3a2f5081dc8 (Analyze actions, Analyze JavaScript/TypeScript, Scorecard file checks, and HTML/scripts validation). I did not make a live GraphQL request because it requires credentials.
What's good: the latest change closes the previously unsafe display-name path by rendering hostile Markdown/HTML syntax as escaped text, and its focused regression test covers image, HTML, link, and newline payloads.
sprayberry-secondread
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the Claude second-opinion lane (independent second read; the gating review is posted separately).
Verdict: No blocking issues at 6e56cb5. One medium efficiency point in the workflow and one low test gap. Neither changes behaviour a user sees.
Medium: .github/workflows/sponsors-readme.yml, a pending bot PR gets force-pushed again every day
.github/workflows/sponsors-readme.yml:52 and :70-74
if git diff --quiet -- README.md; then
...
git commit -m "README: sponsors block refreshed $(date -u +%Y-%m-%dT%H:%MZ)"
...
git push --force origin "$branch"The change check compares the rebuilt block against main, not against bot/sponsors-readme. So when a bot PR is open and not merged yet, every daily run still sees changed=true, makes a new commit (the timestamp in the message gives it a new sha even when the tree is the same) and force-pushes it. When AMNESIA_BOT_PAT is set, which the header and README recommend, each push is a real synchronize: CI runs again, and review-kick.yml (pull_request_target: synchronize) dispatches a fresh review round for a README change that is identical to yesterday's. It keeps happening until someone merges the PR. Without the PAT, nothing is triggered and the only cost is the rewritten branch.
Suggested fix: skip the push when the open branch already has this README.
git commit -m "README: sponsors block refreshed $(date -u +%Y-%m-%dT%H:%MZ)"
if git fetch --depth 1 origin "$branch" 2>/dev/null && git diff --quiet FETCH_HEAD HEAD -- README.md; then
echo "bot/sponsors-readme already carries this block"
exit 0
fi
git push --force origin "$branch"Low: test/sponsors.test.mjs:71-74, the pagination guard test can't tell its two clauses apart
scripts/sponsors.mjs:117
if (!page.endCursor || page.endCursor === cursor) throw new Error('GraphQL pagination did not advance');The test's first page returns hasNextPage: true, endCursor: null while cursor is still null, so both !page.endCursor and null === null are true. Delete either clause and the test still passes. It only fails when the whole guard is gone. Each clause covers a different malformed response, and each is untested on its own:
- page 2 returns the same non-null cursor it was given (
'c1'→'c1'): without=== cursorthis loops forever; - page 2 returns
hasNextPage: true, endCursor: null: without!page.endCursor,cursorgoes back tonulland the loop starts again from page 1.
These only happen if the GitHub API misbehaves, so this isn't blocking. It's two lines to pin:
const pages = [page([], true, 'c1'), page([], true, 'c1')];
await assert.rejects(fetchSponsors('askalf', async () => pages.shift()), /did not advance/);In the same spirit, (n.tier && n.tier.isOneTime) at scripts/sponsors.mjs:55 has no test. The one-time case in only recurring sponsors... sets only isOneTimePayment.
Boundaries I rebuilt from the diff
| Row | Code does | Pinned by |
|---|---|---|
nodes undefined / non-array |
[] |
normalizeSponsors drops... (undefined) |
node null, sponsorEntity: null, login: '' |
dropped | same test |
name whitespace-only |
null, so no parenthetical |
not pinned (trivial) |
monthly 25 / 24 |
named / not named | only recurring... (edge / under); > for >= fails it |
one-time via isOneTimePayment / via tier.isOneTime |
excluded / excluded | pinned / not pinned (above) |
| tie on tier | login order | ada, cal |
| zero named sponsors | 3-line block | with nobody to name... |
| markers missing / END before START | throws | missing pinned; b < a not pinned |
hostile display name (image, tag, link, newline, $&) |
escaped, one line; slice-concat so $& is inert |
a sponsor display name...; removing escaping or \s+ collapse fails it |
name containing <!-- sponsors:end --> |
<, !, - escaped, so it can't fake the marker on the next run |
by the escape set (not a dedicated test) |
| pagination: 2 pages | both read, merged | fetchSponsors follows every page |
| pagination: non-advancing cursor | throws | clauses not separated (above) |
failed read with --write |
exit 1, no write, and set -e stops the step |
by code (main catch) |
PR-body claims checked against the diff
- The footer link is a plain
<a>with notarget, script or image.src/_headershasReferrer-Policy: same-origin, so the cross-origin navigation sends no referrer. The self-host image sendsno-referrer(image/amnesia_app.py:96), which is stricter. ✔ - CSP hash regenerated: the results footer sits inside the inline script, so the hash had to change. The
script-srchash insrc/_headerschanged, and CI'scsp-hashes.mjs --checkpasses. The self-host image hashes its inline blocks at runtime (image/amnesia_app.py:61-78). ✔ - Private sponsors never named:
includePrivate: falsein the query. ✔ - PAT fallback for push and
gh pr create:AMNESIA_BOT_PAT || GITHUB_TOKENon checkout and on the PR step, and the GraphQL read usesGITHUB_TOKEN. The ported dario workflow does the same read withGITHUB_TOKENand its last 5 scheduled runs succeeded, and since--writeexits 1 on a failed read, that success means the read worked. ✔ - Workflow surface: triggers are
scheduleandworkflow_dispatchonly, gated ongithub.repository, with no PR-controlled input. The persisted credential is only seen by code from the default branch. ✔
CI: tests, HTML/script validation, CodeQL, Scorecard and build/smoke all pass. The failing kick job is review-kick.yml failing to start review-dispatch.service on the runner ("Interactive authentication required"). That is runner infrastructure, not this diff.
What's good: the display-name escaping is the right defence. The hostile-name test does fail without it, as the body says. Using slice concatenation in replaceReadmeBlock rather than String.replace also avoids $-pattern surprises from sponsor names.
Read: the full diff, the workflows and image CSP/Referrer code at head, and dario's sponsors-readme.yml and its run history. Did not run the test suite (CI is the signal).
SECOND READ: READY
The daily run compared the block against main only, so an unmerged bot PR was force-pushed every day and re-ran its checks and reviews for the same README. It now skips the push when the open PR's branch already carries the block. Tests also pin each half of the pagination guard and the tier.isOneTime exclusion.
sprayberry-secondread
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the Claude second-opinion lane (independent second read; the gating review is posted separately).
Verdict: no blocking issues at 28eea9a. The new commit's skip-if-unchanged logic is correct on every path I traced, and the two test gaps I raised at 6e56cb5 are now pinned by tests that fail without the code they cover.
What changed since my read at 6e56cb5
compare 6e56cb5...28eea9a has 3 commits: the merge of main, which brings in #84's review-kick.yml removal and is not part of this PR's diff; sponsors: don't re-push an open bot PR that already has the block; and the test additions. Files touched: .github/workflows/sponsors-readme.yml and test/sponsors.test.mjs. The rest of the diff is unchanged from my last read, so the boundary table and claim checks there still hold.
Boundaries for the new skip guard (.github/workflows/sponsors-readme.yml, "Open or refresh the PR")
if [ -n "$existing" ] && git fetch --depth 1 origin "$branch" 2>/dev/null \
&& git diff --quiet FETCH_HEAD HEAD -- README.md; then| Row | Code does |
|---|---|
| no open bot PR (none ever, or a stale closed-PR branch left over) | short-circuits, force-pushes, gh pr create |
| open PR, fetch of the branch fails | falls through and pushes. That's safe: it's the pre-commit behaviour |
| open PR, branch README == rebuilt README | skips the push, exits 0. This is the fix |
open PR, same block but main's README changed elsewhere |
diff is non-empty, so it force-pushes a branch rebuilt on current main. That's the right refresh, since it avoids a stale-base conflict |
| open PR, block changed | force-pushes, prints refreshed #N |
gh pr list errors |
set -euo pipefail fails the step. It doesn't take a wrong branch |
git diff --quiet returning 1 inside the if list does not trip set -e. There's no unit test for this shell path, which is acceptable for a workflow step. actionlint and Scorecard are in CI, and both pass.
Tests: can each assertion fail?
fetchSponsors fails rather than loop...: therepeatcase (c1,c1) only throws throughpage.endCursor === cursor. Without that clause, the thirdshift()returnsundefined, aTypeErroris thrown and/did not advance/fails. Thelostcase (c1, thennull) only throws through!page.endCursor. Without it,null === 'c1'is false and the sameTypeErrorfollows. So each half of the guard is pinned separately. The first case (nullat cursornull) is covered by either clause alone, which is fine because the other two isolate them.only recurring sponsors...:tieroncesets onlytier.isOneTime, so removing(n.tier && n.tier.isOneTime)atscripts/sponsors.mjs:55names it anddoesNotMatch(/…|tieronce/)fails.oncestill pinsisOneTimePayment. The static block text contains none ofunder|small|once, so the regex has no false match.
Non-blocking suggestion (pre-existing, not introduced here)
If the sponsor list goes back to what main already has while a bot PR is open (a sponsor cancels before the PR merges), changed=false skips the whole step, and the open PR keeps proposing a list that names the lapsed sponsor. It needs human review to merge, so nothing ships by itself. Still, closing it is cheap:
# suggested: in the sync step's unchanged branch
gh pr list --head bot/sponsors-readme --base main --state open --json number --jq '.[].number' \
| xargs -r -I{} gh pr close {} --comment "Sponsor list matches main again."(The token would need pull-requests: write in that step, which the job already has.)
CI at head: tests, HTML/script validation, CodeQL (actions and JS), Scorecard, build/smoke-test and Cloudflare Pages all pass. The earlier kick failure is gone now that #84 removed that workflow.
What's good: the fix targets the real cost, daily force-pushes re-running checks and reviews on an unchanged bot PR, and fails open to the old behaviour whenever the comparison can't be made.
Read: the full PR diff at head, the 6e56cb5...28eea9a compare and its commit messages, and gh pr checks. Did not run the test suite (CI is the signal).
SECOND READ: READY
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Verdict: approve — no blocking issues found in the live-head change.
I reviewed the sponsor GraphQL pagination and failure paths, tier/one-time filtering, Markdown escaping, README replacement markers, and the scheduled workflow's token, PR-refresh, and no-op behavior. The new regression tests exercise the $25 boundary, pagination cursor failures, empty data, and hostile display names. Required CI is green at 28eea9a56b9d0065664330060991035cf1f4ed5a (Analyze actions/JavaScript, Scorecard file checks, and HTML/scripts validation).
What's good: the workflow fails before modifying the README when the sponsor read fails, and its open-PR comparison prevents repeated force-pushes when the generated README block is already present.
What
Before:
.github/FUNDING.ymlwas the only mention of sponsorship. Neither the site nor the README said how amnesia is paid for.After:
<a>with no script, image or request.Referrer-Policy: same-originmeans GitHub doesn't learn the visitor came from amnesia. The CSP hash is regenerated (src/_headers), and the self-host image computes its own.## Sponsorsection before## Project.scripts/sponsors.mjsis ported from dario's and limited to the README block. It reads every page of the public sponsor list over GraphQL and names recurring sponsors at $25/month and up, which is the promise the Sponsors tiers make. Display names are escaped, so a sponsor can't put Markdown or HTML (an image beacon, say) into the README..github/workflows/sponsors-readme.ymlruns it daily at 06:53 UTC and opens or refreshes abot/sponsors-readmePR when the block changes. It never pushes tomain, and private sponsors are never named.gh pr createuseAMNESIA_BOT_PATwhen that secret is set, so the bot PR gets the required checks. Without it they fall back toGITHUB_TOKEN, and the bot PR needs a close and reopen to start them. The workflow header and the README say so.Why
amnesia promises no ads and no tracking, so sponsors are how it pays for itself. Until now a visitor had no way to find that out, from the site or the repo.
Verification
npm test: 57 pass, 7 of them in the newtest/sponsors.test.mjs: node filtering and order, the $25/$24 cutoff and one-time exclusion, the empty-list sentence, the README markers, pagination across two pages, failing on a cursor that doesn't advance, and a hostile display name (image, HTML, link, newline) rendering as text. The cutoff and escaping tests were each checked by breaking the code (>=to>, escaping removed) and watching them fail.node scripts/csp-hashes.mjs --checkandnode scripts/check-readme-links.mjspass.actionlintis clean on the new workflow. Scorecard 5.5.0--local(the CI job's own check) keeps Pinned-Dependencies, Token-Permissions and Dangerous-Workflow at 10.workflow_dispatchrun will exercise it, and--writeexits 1 on a failed read, so it can't open a PR with an emptied list.Checklist