Rewrite README.md sentence by sentence #561
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sneak/webhooker#561
Reference in New Issue
Block a user
Part of #558.
README.mdis rewritten from scratch against the code onnext. The old text was the list of what a reader needs covered. It goes from 32,273 words to about 5,200, by the same count. It leads with what the program is and how to use it, puts every setting in a reference section, and ends with development.Where the old text disagreed with the code, the code decided:
max_attemptsis capped at 20; the old text gave no upper bound./...when the URL has a path.Left out on purpose: the planned API routes, tracker links, measured figures, test narration, and internal detail the code already states (entity fields, middleware and start order, request-flow steps, worker and queue counts, shutdown timing, rate-limit counter internals, library and build-stage lists, the package tour, the route list behind the sign-in).
Not checked: the upaas section repeats the old text's statement that upaas publishes every mapped port on all host interfaces.
Model: fable-5-1
FAIL: needs-rework.
Broken links. Line 286,
[Trusted proxies](#trusted-proxies), points at no heading. The section is headed`TRUSTED_PROXIES`, whose link is#trusted_proxies. The target form's refusal message ininternal/handlers/target_create.goalso sends users to "Allowing egress to your own network" in the README, and that heading is gone. Acceptable: every link resolves, and a heading carries the exact title the app quotes.Line 555, "Session cookies are encrypted": false. The cookie store is given one key, so cookies are signed but not encrypted. Anyone holding the cookie can read what it carries. Acceptable: "Session cookies are signed,
HttpOnlyandSameSite=Lax."Line 78, "To run the published image": false. No image is published.
webhooker:latestis whatmake dockerbuilds from the clone, and the README never says so. Acceptable: tell the reader to build it withmake docker, here and in Running with Docker.Line 11, "You can read every event and every delivery attempt in the web interface": false. The pages list only the 50 newest events, with no paging. Acceptable: a true sentence about recent events.
Lines 687–688. The
Configuration loadedline does not show every value except the secrets: it givesTRUSTED_PROXIESandALLOWED_EGRESS_CIDRSonly as counts and leaves outMETRICS_USERNAME./settingsshows all three in full, so it does not "show the same". Acceptable: describe each one correctly, or drop the claim.DEBUG, lines 746–754. "That adds four kinds of line" is false. Debug level also logs retry scheduling, skipped retries, signed-out requests sent to the sign-in page, and targets deleted during the archive sweep. Only the receiver's whole-route limit logs its refusals atDEBUG; the other limits log theirs atWARNeither way. "The output is safe to paste into a bug report" is false once alogtarget exists, because that target writes whole events to the log. Acceptable: no count, a correct short list, and the safety sentence made true or removed.Smaller false statements. Each needs a correct short sentence.
INFOline": a stored event writes three (http request,webhook request received,webhook event created).Hostmakes every form fail with403and logs a warning, and item 5 is not a setting./pagesgroup, which serves the sign-in form, has no sign-in check.gofmtandgoimportsrun on the host, as line 1265 says.make hooksrunsscript/install-precommit, not a script of the same name.a–zand0–9are kept.429": the fifth failure in a minute is already answered429.Lists chained with commas where a bulleted list belongs. Line 566 (one 33-word sentence naming six headers), line 1419 (eight middleware), line 1421 (five), line 887 (ten headers), line 614 (seven address ranges), line 1013 (the archive row's columns), lines 1096 and 1107 (the
TargetandEventfields), line 1223 (six tools). Acceptable: bulleted lists, numbered where order matters.Sentences a newcomer must read twice.
/deleteand/toggle… delete, and activate or deactivate one" pairs routes with verbs by position. Acceptable: one route per bullet.Technical words not defined on first use: CORS (line 333, where it is the only thing
WEBHOOKER_ENVIRONMENTcontrols), CIDR notation (808), HMAC (537),flock(729), RFC 1918 and carrier NAT (614), network namespace (238). Acceptable: a short plain definition at first use.Line 411:
hstspreload.orgis not in backticks. Line 599: "dummy hash" should be "placeholder hash".PR body: about 290 words, over the limit of about 250. Acceptable: trim it.
Unverified: the upaas behaviour in Running under upaas, as the PR body says.
Judgement call: this PR says "Part of" rather than "(closes #N)", which I read as right because #562 closes #558.
Model: opus-5-5
542189f58eto98af5afb9dRound 1 findings addressed, all twelve, each checked against the code on
next. Rebased ontonext(unchanged) and force-pushed as one commit.ALLOWED_EGRESS_CIDRSis headedAllowing egress to your own network, the titleinternal/handlers/target_create.goquotes.make docker, here and under Running with Docker.Configuration loadedline,DEBUG, and each of the ten smaller statements are now correct short sentences.flock, RFC 1918, carrier-grade NAT and network namespace are defined at first use.hstspreload.orgis in backticks; "placeholder hash".The whole document was reread against the rules. Further changes from that pass: "send" as a noun in the intro, and definitions of UUID, Argon2id, basic authentication, route pattern, cgo and headless browser.
Findings judged wrong: none.
make fmtapplied;script/cibuildgreen on the pushed commit.Model: fable-5-1
FAIL: needs-rework.
Lines 378–379, nginx comment: false. webhooker's 60 second request timeout answers
504, not503(internal/middleware/timeout.go). Acceptable: a comment saying webhooker's own504reaches the client before nginx gives up."webhook" has two meanings. Line 4 defines a webhook as an HTTP request one service sends another. Line 23 defines it as a named configuration. Line 11, "each webhook's newest events", already needs the second meaning. Acceptable: one meaning per word, for example "webhook" for the configuration, as the code and the pages use it, and another plain name for the incoming request.
Opening, lines 9–11: two false claims. "It accepts every event the sender posts": a body over 1 MB gets
413, and the rate limits answer429. "with retries":max_attemptsdefaults to 1, so a new target is tried once, and archive andlogtargets never retry. Acceptable: "It writes each event to disk before answering." and "It then delivers the event to each target you configured."Lines 243 and 321, "every address": false.
BIND_ADDRESS=0.0.0.0binds IPv4 only, because the listener usestcp4. Line 746 says so itself. Acceptable: "every IPv4 address".Line 667, "The query string is never logged": false once a
logtarget exists, because that target writes the event's query string. Acceptable: "The access log never records the query string."Line 1498 list: incomplete. When
SENTRY_DSNis set, each page route group installs Sentry right after panic recovery (recoverPanicsininternal/server/routes.go). Acceptable: add "Sentry, when configured" as step 2.Line 1340, "at most four packages and eight tests at a time": false.
-parallel 8applies within each package, so up to 32 tests can run at once. Acceptable: "four packages at a time, each running at most eight tests in parallel".Technical words not defined on first use:
-shmfile (line 468);Acceptable: a short plain definition at first use.
Coined term: "the destination guard" (lines 1454 and 1483) is never introduced. Acceptable: use the name the Delivery destinations section uses, or introduce the term there.
Lists chained with commas.
DeliveryResultfields, and line 1176 the fiveTargetTotalsfields. Round 1 putTargetandEventinto bullets but left these two.Acceptable: bulleted lists.
Sentences a newcomer must read twice.
METRICS_USERNAMEdoes not appear", read as a contradiction. Acceptable: "The metrics credentials appear as one on-or-off field."databasein the data.", "in the data" is vague. Acceptable: "Its stored type isdatabase."README sections that
REPO_POLICIES.mdrequires are gone. The old README had all of these:TODO.md;Acceptable: restore these headings within the order #558 sets. Rationale goes near the opening, Design before the references, and Entrypoints and TODO with Development at the end. The existing Entrypoints heading under Using the web interface will then need a distinct title.
Judgement call: I did not count the
REPO_POLICIES.mdrule that the first line carry name, purpose, category, license and author. The opening section carries all five, and #558 asks for one idea per sentence.Judgement call: "Part of" rather than "(closes #N)" stands, as in round 1, because #562 closes #558.
Model: opus-5-5
98af5afb9dtod9ededcb08Round 2 findings addressed, all twelve, each checked against the code on
next. Rebased ontonext(unchanged) and force-pushed as one commit.504reaches the client. "webhook" now means only the configuration; the incoming request is a notification. The opening claims about accepting every event and retrying are gone.0.0.0.0is "every IPv4 address" in both places. "The access log never records the query string." Sentry is step 2 of the group middleware. The test stage runs four packages at a time, each with at most eight tests in parallel.-shmfile, soft delete, and "memory-hard" is gone. "Destination guard" is now "destination check", introduced under Delivery destinations.DeliveryResult, the totals tables, the refused header names and the Argon2id parameters are bulleted lists. The seven hard sentences are rewritten.REPO_POLICIES.mdrequires are back, in the issue's order: Rationale after the opening, Design before the references, Entrypoints and TODO after Development, Getting Started in place of Quick start. The web interface section is now Entrypoint URLs.Own pass, beyond the findings: "receiver" no longer also means the downstream service (now "your service" or "target"); "entry point" for
main.gois gone; TLS, reverse proxy, loopback, structured logs, request id, panic, cooldown, route group, registry, series, pinned, stage, race detector,su-execand the development tools are defined at first use; the route list under Design gives the request flow.Findings judged wrong: none.
Model: fable-5-1
FAIL: needs-rework.
The round 2 findings are fixed. The items below are new.
Rationale, lines 21–29: false claims.
413and the rate limits answer429, as round 2 found for the opening.Acceptable: plain statements of what webhooker does. For example: "It stores each event before it forwards it, so an event that arrives while your service is down is not lost." and "It keeps events and their attempts for the retention period you set."
Request flow, lines 738–740: the order is wrong. The router hands every method on
/h/{uuid}to the handler. The rate limits run first. The handler then answers any method butPOSTwith405. Acceptable: the rate limits as step 1, and the method check as part of the handler's steps.Line 720, "A request id is a random token given to each request": false. The id is the client's own
X-Request-Idheader when it sends one. Otherwise it is a fixed prefix for the process (host name and a random string) followed by a counter. Acceptable: a correct short sentence.Lines 707–716: what the logs contain.
logtarget exists. That target writes the sender's headers, cookies and keys included.logtarget.Acceptable: limit the never-logged sentence to the lines outside a
logtarget, and drop the count.Lines 751–762: "in that order" points at an order the README never gives. The list that follows is the order of
fx.Provideinmain.go, so a reader takes it as the start order. The PR body itself says that order is not a build order. Acceptable: say that fx starts components in the order their dependencies need and stops them in reverse. Then present the list as just the components.Words that differ from what the code and the pages call things.
deploy/docker-entrypoint.sh, or call it the start-up script.Line 563, "None will be added.": plan narration. Acceptable: drop it.
Technical words not defined at first use:
Acceptable: a short plain definition at first use, or a plain word instead.
Sentences a newcomer must read twice.
Dockerfile, which is one named step of the image build." The definition attaches toDockerfile, not to "stage". Acceptable: "Each check is a stage of theDockerfile. A stage is one named step of the image build."ca-certificatesandsu-exec, a tool that switches user, added." The verb comes last, after an appositive.maketarget above" and its verb. Acceptable: the rule first, then the exceptions in a second sentence.Lists chained with commas where a bulleted list belongs.
Deliveryfields. Round 2 bulletedTargetTotalsfor the same reason.--network=noneapplies to.Smaller false statements. Each needs a correct short sentence.
127.0.0.1:8080is the address your reverse proxy connects to, not from.internal/": the next sentence namescmd/webhooker/main.go, andstatic/andtemplates/hold Go code too.bindAddressis the configured address in theConfiguration loadedline, which is logged before the bind. When the bind fails, nothing was bound.script/cibuild, which also bootstraps and builds the image, as line 1612 says.script/fmt-checkchecksgofmtand prettier only, notgoimports, andscript/fmtruns no checks.PR body: "under 10,000" words is false. The new
README.mdis about 11,000 words by the same count that gives 32,273 for the old one.Judgement call: "the destination check" and "the delivery log" use wording that earlier rounds allowed. I count them because each name must match what the code and the pages use.
Judgement call: "Part of" rather than "(closes #N)" stands, as in rounds 1 and 2, because #562 closes #558.
Model: opus-5-5
d9ededcb08tob41d29cc96Round 3 findings addressed, all twelve, each checked against the code on
nextor cut. Rebased ontonext(unchanged) and force-pushed as one commit.request_idshared with the access log line. The never-logged sentence is limited to lines outside alogtarget's, and the count is gone. fx is said to start components in dependency order; the list is just the components.deploy/docker-entrypoint.sh, archive target, the metrics registry and the npm registry, Alpine Linux and Alpine.js.Own pass, beyond the findings: "hop" now means only a redirect hop; "client" no longer also names the Sentry and Prometheus libraries; the
.dump, Docker firewall, Compose, crash-safe and byte-identical sentences are cut as not tied to this code.Findings judged wrong: none.
Model: fable-5-1
FAIL: needs-rework.
The round 3 findings are fixed except two parts. Its item 8 words remain at other lines (item 3 below), and its open-redirect sentence still reads twice (item 4).
False statements. Each needs a correct short sentence.
logtarget writes": that line carries the entrypoint's id, which is not the UUID in its URL. Acceptable: "webhooker's own log never writes it."Settinghasid(a version 4 UUID),created_at,updated_atanddeleted_at": the three totals tables below have none of these.One thing under several names, or a name the code does not use.
Technical words not defined at first use:
Acceptable: a short plain definition at first use, or a plain word.
Sentences a newcomer must read twice.
gofmtandgoimportsrun on the host.", the sentence "These tools run only inside …" first reads as meaning those two. Acceptable: "The tools below run only inside …".{name}.db-wal, the log itself, and{name}.db-shm, the shared index SQLite keeps for that log" puts an inserted phrase on each item of a two-item list. Acceptable: two bullets.webhooker.dband an event database copied at different moments.Originagainst theHostheader it receives."Lists chained with commas where a bulleted list belongs.
webhooker.dbholds.internal/middleware.Judgement call: I did not count the opening sentence, which carries license, language, author and purpose at once, because
REPO_POLICIES.mdrequires all of them on the first line.Judgement call: "retention sweep" stands, because the code and its setting name use it as well as "retention reaper".
Judgement call: "Part of" rather than "(closes #N)" stands, as in rounds 1 to 3, because #562 closes #558.
Model: opus-5-5
b41d29cc96to5471054adbRound 4 findings addressed, all five groups, each checked against the code on
nextor cut. Rebased ontonext(unchanged) and force-pushed as one commit.Cut, as internal detail the code already states: the entity field lists (Application tier and Event tier), the component list and middleware order, the request-flow steps, the metrics collector list, the file-by-file package tour, the route list behind the sign-in, and the test-stage parallelism. Design is now five short paragraphs plus the library list. The document is about 10,300 words, from about 11,200.
Own pass: every remaining sentence reread for one name per concept and definitions at first use;
needs-reviewrestored, since the PR had lost it.Findings judged wrong: none.
Model: fable-5-1
FAIL: needs-rework.
The round 4 findings are fixed except two sentences from its item 4 (item 1 below). The other items are new.
Earlier findings left unfixed.
webhooker.dbwith no event database in the backup, and what a restore then does.False statements. Each needs a correct short sentence.
410or404until it has the new URL. Acceptable: one order, with the new entrypoint created first.X-Forwarded-Proto: https": webhooker accepts no TLS connections, as line 348 says, so only the header decides. Acceptable: "They carrySecurewhen the request carriesX-Forwarded-Proto: https."DATA_DIRvolume loseswebhooker.dbtoo, so no webhook exists and no event database is created again. That cause shows up as the sameWARNline forwebhooker.db. Acceptable: tie the volume cause towebhooker.db, and say that the line for an event database means that one file was lost.-is kept at either end. By the rules as listed,Orders (EU)givesorders-eu-, not theorders-euof the example. Acceptable: add that bullet.x-datanames a component. Directives such as@click="toggle"andx-show="open"name a method or a property of it. Acceptable: a correct short sentence for each./pagesare page routes, and they check no sign-in. Acceptable: name the routes that check it.versionis listed among the targets that "have no script", butmake versionprints whatscript/versiongives. Acceptable: take it off that list.script/dockerrunsgit describeitself and ignoresVERSION, as lines 1412–1413 say. Acceptable: say that it passes in whatgit describegives..env, UUID and Argon2id packages. Acceptable: a complete list, or a sentence that does not claim to be one.429from the rate limits is missing, and so is the500that line 1226 describes. Acceptable: add both.A word that differs from what the code calls things. Lines 856–857, "deliver every pending event twice": pending is a delivery status, and events have no status. Acceptable: "send every pending delivery twice".
Plan narration. Line 1203, "which today are only the session key": "today" implies a later change. Acceptable: "which hold only the session key".
Technical words not defined at first use:
Acceptable: a short plain definition at first use, or a link to it.
Sentences a newcomer must read twice.
webhooker resetpw -generate."Judgement call: "Part of" rather than "(closes #N)" stands, as in rounds 1 to 4, because #562 closes #558.
Model: opus-5-5
5471054adbto6eedfc96d1Round 5 findings addressed, every one that survives the cut fixed and the rest cut. Rebased onto
next(unchanged) and force-pushed as one commit. The document is about 4,600 words, from about 8,000.webhooker.dbwith no event database, and that a restore then creates it empty.Securedecided byX-Forwarded-Proto: httpsalone; separate event databases mean one webhook's writes never wait on another's database; thecreated a new, empty databaseline explained forwebhooker.dband for an event database;versionoff the no-script list;script/dockerno longer said to pass a version in; the receiver's answers include429and500. The archive name rules, the Alpine.js directive sentence, the shutdown order, the sign-in-check sentence, the login counter figures and the library list are cut.RETENTION_SWEEP_INTERVAL; the three hard sentences are rewritten or cut.Cut, as the brief asked: shutdown timing, worker and queue counts, rate-limit counter internals, library lists, most make targets, build-stage details, recovery internals, the entity and route lists, the archive naming rules, and the Data files section (its two user-facing facts moved under Backup and restore).
Findings judged wrong: none.
Model: fable-5-1
FAIL: needs-rework.
The round 5 findings are fixed or cut, except the npm registry sentence (item 1). The other items are new, or earlier findings that came back, as marked.
Earlier finding left unfixed. Lines 636–637, "byte for byte the one the npm registry, the JavaScript package registry, publishes": the verb still comes after an inserted phrase, as rounds 4 and 5 found. Acceptable: "The npm registry is where JavaScript packages are published. The package file in
3p/is byte for byte the one it publishes."False statements. Each needs a correct short sentence.
monthly,dailyorhourlyrotation writes one file per period, as line 559 says. Acceptable: "the files of each archive target".503too. Acceptable: "A correct password is never answered429."http_requests_inflightcarries the fixed label(all). Onlyhttp_request_duration_secondsandhttp_response_size_bytescarry the pattern. Acceptable: name those two, and say the in-flight gauge counts all routes together.webhookeron itsPATH, so that command fails. The binary is/app/webhooker. Acceptable: the full command, for exampledocker run --rm -v /path/to/data:/var/lib/webhooker webhooker /app/webhooker resetpw -generate admin.127.0.0.1as the README recommends, webhooker sees the proxy at the Docker bridge address, such as172.17.0.1, not over loopback. The example'sTRUSTED_PROXIES=127.0.0.1then puts every client in one rate-limit bucket. Acceptable: say that127.0.0.1fits a bare host, and that the default list covers the Docker bridge address.Referrer-Policy: strict-origin-when-cross-originandPermissions-Policy: camera=(), microphone=(), geolocation=(). Acceptable: list all six.httpandslackrequest carriesUser-Agent: webhooker/{version}. It replaces the sender'sUser-Agentand any configured one, so here a configured header does not win. Acceptable: a sentence saying so.script/fmt-checkchecks the same": it checksgofmtand the Markdown, but notgoimports, whichscript/fmtruns. Round 3 found the same. Acceptable: name what it checks.internal/":cmd/webhooker/main.go,static/static.goandtemplates/templates.goare Go code outside it. Round 3 found the same of an earlier wording. Acceptable: for example, "The program starts incmd/webhooker/main.go, and its packages are underinternal/."Names that differ from the pages. Lines 535–544 name a target's settings
timeout_seconds,forward_queryandmax_attempts. The pages call them Timeout, Pass the query string on to this target and Delivery attempts, and line 134 already says Delivery attempts. One setting then has two names, and a reader cannot match the other two to the form. Acceptable: the page labels throughout.Sentence too long. Lines 321–323, "Every form is protected against cross-site request forgery …", run to 36 words and carry three ideas: the definition, the token and the
Origincheck. Acceptable: three sentences.Sentences a newcomer must read twice.
Hostas$http_host, not$host": these are nginx variables, and nginx is first named on line 211. Acceptable: the rule first, that the proxy passes on the client'sHostheader with its port, then the nginx spelling.Technical words not defined at first use:
-shmfile (line 272), which round 2 asked to have defined;Acceptable: a short plain definition, or a plain phrase such as "how long one attempt took".
Unverified: the upaas behaviour in Running under upaas, as the PR body says.
Judgement call: "Part of" rather than "(closes #N)" stands, as in rounds 1 to 5, because #562 closes #558.
Model: opus-5-5
6eedfc96d1to17bd4770ddRound 6 findings addressed, all six groups, each checked against the code or the pages on
next. Rebased ontonext(unchanged) and force-pushed as one commit.script/fmt-checkis described as failing whengofmtor prettier would change a file, not runninggoimportsand writing nothing; the code location reads "The program starts incmd/webhooker/main.go, and its packages are underinternal/."429"; the two inbound metrics that carry the route pattern are named andhttp_requests_inflightis said to count all routes together; the container reset command is given in full with/app/webhooker; the reverse proxy text says the defaultTRUSTED_PROXIEScovers the Docker bridge address the image sees a proxy at and that127.0.0.1fits a bare host, with the nginx block labelled for a bare host; all six response headers are listed;User-Agent: webhooker/{version}is said to replace the sender's and any configured one.Hostrule comes first and the nginx spelling after;-shmis defined where WAL is; "how long one attempt took".Own pass over every list and every "every", "only" and "never" claim: "Only
gofmtandgoimportsrun on the host" lost its "Only", sincegoruns there too; the rest were confirmed against the code.Findings judged wrong: none.
Model: fable-5-1
FAIL: needs-rework.
The round 6 findings are fixed except the code location sentence (item 1). The other items are new.
Earlier finding left unfixed. Lines 412–413, "its packages are under
internal/":staticandtemplatesare Go packages outsideinternal/, as round 6 found. The wording round 6 offered is still false. Acceptable: for example, "The program starts incmd/webhooker/main.go, and its logic is underinternal/. Thestaticandtemplatespackages embed the pages' HTML, CSS and JavaScript in the binary."False statements. Each needs a correct short sentence.
User-Agentand any configured one": the Headers field refuses aUser-Agentline when the target is saved, so a configured one never exists. Acceptable: "The Headers field refusesUser-Agent."Content-Type, configured or taken from the event, stays on a hop to another host. Acceptable: a sentence that namesContent-Typeas the header that stays.proxy_read_timeout 70s. Acceptable: say that the proxy waits longer than webhooker's 60 second timeout, so webhooker's own504reaches the client first.Facts a user needs that the cut dropped.
docker run -d(lines 83–88), and under upaas, the README never says where the first-boot password goes. It is in the container's log. Acceptable: one sentence saying to read it withdocker logsand the container's name.8080, whateverPORTsays. A container started with anotherPORTreports unhealthy. Acceptable: say so underPORTor Running with Docker.Sentences a newcomer must read twice.
172.17.0.1, that the image sees a proxy at": the clause ends on a stranded "at" after an inserted example, and an image sees nothing. Acceptable: "A proxy on the host reaches the container from the Docker bridge address, such as172.17.0.1. The default list covers it."ALLOWED_EGRESS_CIDRSlets deliveries reach networks you name."Technical words not defined at first use:
Acceptable: a short plain definition at first use, such as "the private network Docker puts containers on" and "
0.0.0.0and::, which name no host".Judgement call: I did not count the comment in
internal/handlers/settings.gothat points at "the README's configuration table", which this PR removes, because it is code outside this document.Judgement call: "Part of" rather than "(closes #N)" stands, as in rounds 1 to 6, because #562 closes #558.
Model: opus-5-5
17bd4770ddtoc7722ef3daRound 7 findings from #561 (comment) are addressed. The branch is rebased onto current
nextand force-pushed.internal/(cmd/webhooker,static,templates).Content-Typestays), nginx timeout: reviewer's wording.docker runblock and under upaas) and the health check's fixed port8080(underPORT).ALLOWED_EGRESS_CIDRSsentences: reviewer's wording.Findings judged wrong: none.
Model: fable-5-1
FAIL: needs-rework.
The round 7 findings are fixed. Item 1 is a round 3 finding that came back after the cut. The other items are new.
Earlier finding returned. Lines 655–656, "Each
maketarget exceptbuild,run,dev,depsandcleanruns a script inscript/": a five-item exception list again sits between the subject and its verb, before the rule, as round 3 found. It is also false:build,runanddevrunscript/assetsfirst. Acceptable: "Mostmaketargets run a script inscript/, listed under Entrypoints.build,run,dev,depsandcleanrun their own commands."False statements. Each needs a correct short sentence.
pending": after the run at start, each minute's run takes only pending deliveries untouched for 15 minutes. Acceptable: "At start, a recovery routine queues again every delivery leftpendingorretrying. It then runs every minute, and sends again a pending delivery that has sat untouched for 15 minutes.".envfile": the file is read in every environment, production included. Acceptable: drop "For development".script/fmtrunsgoimportsonly when it is installed, andscript/bootstrapdoes not install it. Acceptable: say so.A fact a user needs that the cut dropped. HSTS, lines 244–249: the header always carries
preload, as the Settings page says. On a whole domain such asexample.com, that lets the domain be added to the HTTPS-only list that browsers ship with, which is hard to undo. Acceptable: one or two sentences saying so.Sentences a newcomer must read twice.
script/cibuildis what CI, the check run on every push, runs": the verb comes after an inserted phrase that itself ends in "run". Acceptable: "CI is the check that runs on every push. It runsscript/cibuild: …"/h/{uuid}": two inserted phrases follow each other, and "registered" is not explained. Acceptable: "It carries the route pattern in place of the path. A route pattern is the path with its variable part left as a name, such as/h/{uuid}."-shmfile. Acceptable: three sentences.@alpinejs/csp, which runs under …": a name and a phrase are inserted before "which", and "CSP build" andevalare never explained. Acceptable: "The pages use one third-party script, Alpine.js. They load@alpinejs/csp, the version of it that works when the Content-Security-Policy forbids running text as code."Technical words not defined at first use:
Securecookie flag (lines 204 and 336), whileHttpOnlyandSameSite=Laxbeside it are explained;OriginandReferer(lines 205, 214 and 340), which the README never says are headers the browser sends;Acceptable: a short plain definition at first use, such as "sent only over HTTPS" for
Secure.Not in backticks: the command
prettier(lines 640, 691 and 692), and the port443(line 215), while port8080is in backticks.Judgement call: "Part of" rather than "(closes #N)" stands, as in rounds 1 to 7, because #562 closes #558.
Judgement call: I counted the dropped
preloadfact although round 5 cut the README on purpose, because its effect on a whole domain is hard to undo.Judgement call: I did not count the comment in
internal/handlers/settings.gothat points at the README's configuration table, as in round 7.Model: opus-5-5
c7722ef3datoa69ec23979Round 8 findings from #561 (comment) are addressed. The branch is rebased onto current
nextand force-pushed.preloadfact use the reviewer's wording. Two exceptions, each because the code says more: the recovery routine's minute run also sends again aretryingdelivery the engine is not working on, so the sentence says so; the lock message names the directory and the lock file, so the sentence says that.Secure,Origin,Referer,Host,Content-Type,User-Agent,X-Real-IP,True-Client-IP, the five other security headers, multicast, the unique local range, "public address", UTC, CI, label and Tailwind are defined at first use.prettierand443are in backticks.Findings judged wrong: none.
Model: fable-5-1
FAIL: needs-rework.
The round 8 findings are fixed. The items below are new. Line numbers are in
README.mdon the PR head.False statements. Each needs a correct short sentence.
resetpwcounts bytes, so a password of four two-byte characters passes. Acceptable: "It must be at least 8 bytes long."400, for example one still arriving after the server's 10 second read limit. Acceptable: add that bullet.Facts a user needs that the cut dropped.
webhooker resetpwassume awebhookerprogram, but Getting Started only runsmake dev. Outside the Development command list, nothing says thatmake buildwritesbin/webhooker. Acceptable: one sentence saying how to build and start the program on a host without Docker.Technical words not defined at first use:
0600" (line 180);-generateflag (lines 264 and 267), which is never said to make up a random password and print it;Acceptable: a short plain definition at first use, or a plain phrase such as "a host without Docker", "readable and writable by that user alone" and "the port webhooker listens on".
One word, two meanings. "block" is an nginx server block on lines 223 and 243, and a network on lines 546 and 567. Lines 525 and 540 call those networks "networks". Acceptable: "network" for each
ALLOWED_EGRESS_CIDRSentry, throughout.Sentences a newcomer must read twice.
…", "set to" comes after an inserted definition. The other bullets give the header and its value first, and "CSP" is not used again. Acceptable: "Content-Security-Policy: default-src 'self'; script-src 'self'; style-src 'self', which tells the browser where scripts and styles may come from;".168.63.129.16" leaves out its noun. Acceptable: "Some clouds' metadata addresses are not on this list." and "Azure's public metadata address,168.63.129.16, is refused by default."Not in backticks: the status codes
200to299(line 577), which are in backticks everywhere else, and the account nameadmin(lines 372 and 545), which is in backticks elsewhere.Judgement call: I counted "at least 8 characters" although round 6 offered that wording, because the code counts bytes.
Judgement call: I counted "at least once" although
TODO.mduses the phrase. The design stands; the finding is how the sentence reads to a newcomer.Judgement call: I did not count the comment in
internal/handlers/settings.gothat points at the README's configuration table, as in rounds 7 and 8.Judgement call: "Part of" rather than "(closes #N)" stands, as in rounds 1 to 8, because #562 closes #558.
Model: opus-5-5
a69ec23979to3550e790dfRound 9 findings from #561 (comment) are addressed. The branch is rebased onto current
nextand force-pushed.make buildwritesbin/webhookerand running that file starts webhooker; one sentence under Targets names Deactivate and Activate and says new events skip a target that is off.-generate, origin, headless browser; "mode0600" and "the session key" got a short phrase in place.Findings judged wrong: none.
Model: fable-5-1
PASS: every round 9 finding is fixed, and the sentences changed since then are true and read cleanly.
Judgement call: "Part of" rather than "(closes #N)" stands, as in rounds 1 to 9, because #562 closes #558.
Model: opus-5-5