Closes #190: README.md, rewritten from scratch, 20800 words down to about 11300, and checked statement by statement against the code on next. It leads with what the program is and how to use it, puts every setting in a reference section, and ends with development, testing and a TODO section that points at the tracker. No tables; every command, field, numeric reply, endpoint, setting and limit the code has is kept. REPO_POLICIES.md and CONVENTIONS.md are copies of shared documents and are untouched. Reworked against #215 (comment); the rework comment lists the changes.
Where the old text disagreed with the code, and what this says instead:
KICK showed only the client's command. The server sends a KICK event to every member, the kicked user included: kicker in from, channel in to, kicked nick in params, reason in body; written so.
Nicks compared by exact case, channel names unmentioned. Channel names compare by exact case too (#Upper and #upper are two channels), and the HTTP API's 005 still says CASEMAPPING=ascii (#128); written so.
DBURL suggested file::memory:?cache=shared for testing. The one-connection pool replaces its connection after a cancelled query and SQLite then drops the in-memory database; written as a DBURL not to set. The tests use one and hold a second connection to keep it.
Federation, PUBKEY, message signing, canonicalization, TOFU key management and Postgres were described as design or planned. None exists; dropped. meta is relayed unchanged on PRIVMSG and NOTICE, dropped on every other command, and the server reads only meta.hashcash.
FEDERATION_KEY was listed as a setting. The config reads it and nothing uses it; left out.
+n was "shown only". The IRC listener can set and clear it, the HTTP API cannot, and it changes nothing; written so.
"The CLI provides MintChannelHashcash". The function exists in internal/cli/api; neoirc-cli never calls it and sends no channel stamps; written so.
Numeric examples for 324, 329 and 333 had no body. Every numeric carries a body, some one empty string; written so.
GET /api/v1/server showed "name": "My NeoIRC Server" and a version number. name is empty without SERVER_NAME and version empty in a build without one; written so.
"Safari does not" send the Secure cookie to localhost was unverified; dropped.
make build was shown as producing the server; the binary it produces serves an empty page at / unless the web client was built first; written so.
The neoirc-cli text implied that Alt and a digit changes where typed text goes. It does not (#221); the text says what the code does now.
Filed, not in the README: #216 (QUIT reason), #217 (PASS notice) and #221 (neoirc-cli target). The Makefile has no target for neoirc-cli, so the README shows go build.
Not checked: the server was not run. That Chrome and Firefox send the Secure cookie to localhost over plain HTTP is general knowledge, not run here; curl 8.18 was checked by the reviewer. The 404, 405 and 504 answers are read from the router and middleware source (chiv5.2.1), not run. The web client's source was read only for its framework, initChannelState and Web Crypto calls.
Model: fable-5-1
Closes https://git.eeqj.de/sneak/neoirc/issues/190: `README.md`, rewritten from scratch, 20800 words down to about 11300, and checked statement by statement against the code on `next`. It leads with what the program is and how to use it, puts every setting in a reference section, and ends with development, testing and a `TODO` section that points at the tracker. No tables; every command, field, numeric reply, endpoint, setting and limit the code has is kept. `REPO_POLICIES.md` and `CONVENTIONS.md` are copies of shared documents and are untouched. Reworked against https://git.eeqj.de/sneak/neoirc/pulls/215#issuecomment-159165; the rework comment lists the changes.
Where the old text disagreed with the code, and what this says instead:
- `KICK` showed only the client's command. The server sends a `KICK` event to every member, the kicked user included: kicker in `from`, channel in `to`, kicked nick in `params`, reason in `body`; written so.
- Nicks compared by exact case, channel names unmentioned. Channel names compare by exact case too (`#Upper` and `#upper` are two channels), and the HTTP API's `005` still says `CASEMAPPING=ascii` (https://git.eeqj.de/sneak/neoirc/issues/128); written so.
- `DBURL` suggested `file::memory:?cache=shared` for testing. The one-connection pool replaces its connection after a cancelled query and SQLite then drops the in-memory database; written as a `DBURL` not to set. The tests use one and hold a second connection to keep it.
- Federation, `PUBKEY`, message signing, canonicalization, TOFU key management and Postgres were described as design or planned. None exists; dropped. `meta` is relayed unchanged on `PRIVMSG` and `NOTICE`, dropped on every other command, and the server reads only `meta.hashcash`.
- `FEDERATION_KEY` was listed as a setting. The config reads it and nothing uses it; left out.
- `+n` was "shown only". The IRC listener can set and clear it, the HTTP API cannot, and it changes nothing; written so.
- "The CLI provides `MintChannelHashcash`". The function exists in `internal/cli/api`; `neoirc-cli` never calls it and sends no channel stamps; written so.
- Numeric examples for `324`, `329` and `333` had no `body`. Every numeric carries a `body`, some one empty string; written so.
- `GET /api/v1/server` showed `"name": "My NeoIRC Server"` and a version number. `name` is empty without `SERVER_NAME` and `version` empty in a build without one; written so.
- "Safari does not" send the `Secure` cookie to `localhost` was unverified; dropped.
- `make build` was shown as producing the server; the binary it produces serves an empty page at `/` unless the web client was built first; written so.
- The `neoirc-cli` text implied that Alt and a digit changes where typed text goes. It does not (https://git.eeqj.de/sneak/neoirc/issues/221); the text says what the code does now.
Filed, not in the README: https://git.eeqj.de/sneak/neoirc/issues/216 (`QUIT` reason), https://git.eeqj.de/sneak/neoirc/issues/217 (`PASS` notice) and https://git.eeqj.de/sneak/neoirc/issues/221 (`neoirc-cli` target). The `Makefile` has no target for `neoirc-cli`, so the README shows `go build`.
Not checked: the server was not run. That Chrome and Firefox send the `Secure` cookie to `localhost` over plain HTTP is general knowledge, not run here; `curl` 8.18 was checked by the reviewer. The `404`, `405` and `504` answers are read from the router and middleware source (`chi` `v5.2.1`), not run. The web client's source was read only for its framework, `initChannelState` and Web Crypto calls.
Model: fable-5-1
TODO is missing. REPO_POLICIES.md requires it; the issue forbids narrating plans. Acceptable: a short ## TODO section before License that points to https://git.eeqj.de/sneak/neoirc/issues for open work and narrates no plan. The other required sections are present; Rationale as "Why it exists" and Design as "How it works" plus "Layout" are read as meeting the policy under other headings.
False or over-general statements
How it works: "The 005 reply still advertises CASEMAPPING=ascii: IRC has no value for names that compare by exact case." Holds for the HTTP API only; the IRC listener's 005 has no CASEMAPPING token (Connecting lists it right). "still" also narrates history. Acceptable: "The HTTP API's 005 reply advertises CASEMAPPING=ascii; IRC has no value for names that compare by exact case."
How it works: "A client's own commands are echoed to all clients of its session too." Holds for JOIN, PART, NICK, TOPIC, MODE and KICK; an INVITE reaches only the invited user's session, and WALLOPS only sessions with +w. Acceptable: name those six.
GET /api/v1/messages: "It answers as soon as a message arrives for the session, or with an empty messages array when the time is up." The wake is per session and queues are per client: a numeric reply queued to another client of the same session ends this client's wait early with an empty array. Acceptable: "It answers when a message arrives for any client of the session, with an empty array when the message was for another client, or with an empty array when the time is up."
Other paths: "Every other path serves the web client. A path that names one of its files gets that file; any other path gets its index.html." Holds for GET outside /api/v1/: an unknown path under /api/v1/ gets a plain-text 404, and another method on an unregistered path gets 405. With it, Conventions: "An error at the HTTP level is a 4xx or 5xx status with {"error": "<text>"}." Those 404 and 405 answers, and the 504 of the 60-second request timeout, carry no JSON body. Acceptable: scope both sentences to the API handlers' own answers.
Numeric replies: "Sent to every new client, on POST /api/v1/session, POST /api/v1/login and MOTD:" followed by 375, 372 and 376. With an empty MOTD setting the three are not sent and nothing replaces them; the exception is stated only under the MOTD command, and the session and login sections promise "the message of the day" the same way. Acceptable: "375 to 376 only when MOTD is set."
OPER: "ERR_NOOPERHOST (491) when the name or password is wrong, or the server has neither setting." The code refuses when either setting is empty; Configuration says so. Acceptable: "or either setting is empty."
Channel modes, +H: "An IRC listener client cannot send to the channel at all." Only PRIVMSG needs the stamp; an IRC client's NOTICE reaches a +H channel. Acceptable: "An IRC listener client cannot send a PRIVMSG to the channel; its NOTICE still goes through."
Hostmasks: "Only a server operator sees IP addresses, in RPL_WHOISACTUALLY (338), and only that of the user's newest client." When the reverse lookup gives no name, the hostname is the IP address and every user sees it in the hostmask, as the same section says earlier. Acceptable: add "When the lookup gave no name, the hostname everyone sees is the IP address."
Session stamps: "The SHA-256 hash of the whole stamp string must start with at least bits zero bits." The server checks the hash against its own hashcash_bits, not the stamp's bits field; a stamp that claims more than it has passes when it has the required count. Acceptable: "at least hashcash_bits zero bits".
Limits and headers: "Every response carries a Content-Security-Policy header". A cross-origin preflight (OPTIONS) is answered by the CORS middleware before the CSP middleware runs and carries none. Acceptable: "Every response except a cross-origin preflight".
The IRC listener, Commands: "MODE <nick> for another nick gets ERR_USERSDONTMATCH (502), for a query as for a change." Listed as a difference from the HTTP API, which does the same (User modes says so). Acceptable: drop the bullet.
neoirc-cli: "Text without a / goes as a PRIVMSG to the current window's channel or user." After a switch with Alt and a digit, or to window 0, the send target stays the channel or user selected before; only /window, /w, /join and /query change it. Likely a client bug; if the owner fixes the code instead, the sentence stands.
The database: "An in-memory database (file::memory:) is not usable, even for testing." Tests and checks: "A test that needs a database gets an in-memory one of its own from internal/dbtest." The repo's tests do use in-memory databases, kept alive by a held connection. Acceptable: "Do not set DBURL to an in-memory database: ..." and, under Tests, "The tests use one and hold a second connection open to keep it."
Tests and checks: "Tests live next to the code in _test packages." internal/cli/app_internal_test.go and each package's export_test.go are in the package itself. Acceptable: "Tests live next to the code, most in _test packages."
Dropped from the old text while the code still does it
Numeric replies, Errors: "params holds the thing the error is about where the command's section says so". The command sections do not say so, and the old text listed it: for example 401, 432 and 433 carry [nick]; 403, 442 and 482 carry [channel]; 441 and 443 carry [nick, channel]; 421 and 461 the command. A client author needs this to show what an error is about. Acceptable: the params named for each error in that list.
Channel stamps: nothing says that the version, bits and date rules of session stamps apply to channel stamps; the code applies the same checks (48 hours past, one hour future), and the old text said so. Acceptable: one sentence under Channel stamps.
Prose
GET /api/v1/state: "With ?initChannelState=1, the server also adds to the calling client's queue, for each channel, a JOIN event from the session's own nick with an empty body, the topic replies and the member list." 33 words; the object of "adds" arrives after two inserted phrases. Acceptable: "With ?initChannelState=1, the server also fills the calling client's queue as POST /api/v1/login does."
Errors chained with semicolons where a bulleted list belongs (PRIVMSG, JOIN and MODE use bullets for the same thing). TOPIC: "Errors: ERR_NEEDMOREPARAMS (461) without to or body; ERR_NOSUCHCHANNEL (403); ERR_NOTONCHANNEL (442) when the user is not a member; ERR_CHANOPRIVSNEEDED (482) when the channel is +t and the user is not a channel operator." (33 words). INVITE: "Errors: ERR_NEEDMOREPARAMS (461) with fewer than two strings; ERR_NOSUCHCHANNEL (403); ERR_NOTONCHANNEL (442) when the inviter is not a member; ERR_CHANOPRIVSNEEDED (482) on a +i channel; ERR_NOSUCHNICK (401); ERR_USERONCHANNEL (443) when the user is already a member." (36 words). The KICK and KILL "Errors:" lines chain five and four items the same way. Acceptable: one bullet per error.
MODE: "ERR_UNKNOWNMODE (472): a change the list above does not have, a +H value that is not a whole number from 1 to 40, or a +l value that is not a positive whole number." 35 words, three cases. Acceptable: three sub-bullets.
Writing a client: "Show a message by its command: PRIVMSG as <nick> text, NOTICE as -nick- text, JOIN as *** nick has joined #channel, NICK as *** oldnick is now known as newnick, TOPIC as *** nick set topic: text, and a numeric reply by its body." 45 words, six items. Acceptable: a sub-list, one line per command.
The database: "sessions: one row per session: nick, username, hostname, IP address, the server operator and +w flags, the bcrypt password hash, the away message, the creation time and the time of the last request." 34 words, nine items. Acceptable: "nick, username, hostname, IP address, flags, password hash, away message and timestamps".
The message envelope, ts: "UTC, RFC 3339, with fractional seconds and trailing zeros dropped". "dropped" reads as applying to the fractional seconds too. Acceptable: "UTC, RFC 3339 with fractional seconds; trailing zeros are dropped".
"long poll" is never defined. It first appears in the Python comment "longer than the long-poll timeout" and in "Set the HTTP timeout above the long-poll timeout", then under Reverse proxy, Limits and headers and Layout; the behaviour is described under GET /api/v1/messages without the name. Acceptable: name it there: "This wait is a long poll."
Layout: "internal/middleware/: request logging, CORS, CSP and metrics." CORS and CSP are defined nowhere; the text elsewhere says "cross-origin requests" and "Content-Security-Policy header". Acceptable: those words.
Reply numbers after names are bare throughout Commands and Numeric replies, "RPL_TOPIC (332)", "ERR_NEEDMOREPARAMS (461)", while the same numbers are in backticks elsewhere ("such as 353 and 433", "001RPL_WELCOME"). The rule puts every identifier in backticks. Acceptable: "RPL_TOPIC (332)". Judgement call: the number is a gloss of the name; applied because the sibling reviews treated bare reply numbers as findings.
Judgement points asked for
Required sections: see TODO above.
neoirc-cli: "The Makefile has no target for it. Build it from the source tree: go build -o bin/neoirc-cli ./cmd/neoirc-cli." True (the build target compiles only ./cmd/neoircd, and REPO_POLICIES.md requires no build target) and plain; the command needs no web/dist/. Not a finding; a Makefile target is a follow-up for the owner outside this PR.
Not findings here: the QUIT reason, the PASS notice and the schema/ files, filed as #216, #217 and #218. Not checked: browser cookie behaviour (curl 8.18 was checked and sends the Secure cookie to localhost over plain HTTP); the server was not run. Confidence is high on every item except the long-poll early answer and the 404/405/504 bodies (medium: read from the router's and middleware's documented behaviour, not run).
Model: fable-5-1
FAIL, `needs-rework`.
**Required sections**
- TODO is missing. `REPO_POLICIES.md` requires it; the issue forbids narrating plans. Acceptable: a short `## TODO` section before License that points to https://git.eeqj.de/sneak/neoirc/issues for open work and narrates no plan. The other required sections are present; Rationale as "Why it exists" and Design as "How it works" plus "Layout" are read as meeting the policy under other headings.
**False or over-general statements**
- How it works: "The `005` reply still advertises `CASEMAPPING=ascii`: IRC has no value for names that compare by exact case." Holds for the HTTP API only; the IRC listener's `005` has no `CASEMAPPING` token (Connecting lists it right). "still" also narrates history. Acceptable: "The HTTP API's `005` reply advertises `CASEMAPPING=ascii`; IRC has no value for names that compare by exact case."
- How it works: "A client's own commands are echoed to all clients of its session too." Holds for `JOIN`, `PART`, `NICK`, `TOPIC`, `MODE` and `KICK`; an `INVITE` reaches only the invited user's session, and `WALLOPS` only sessions with `+w`. Acceptable: name those six.
- `GET /api/v1/messages`: "It answers as soon as a message arrives for the session, or with an empty `messages` array when the time is up." The wake is per session and queues are per client: a numeric reply queued to another client of the same session ends this client's wait early with an empty array. Acceptable: "It answers when a message arrives for any client of the session, with an empty array when the message was for another client, or with an empty array when the time is up."
- Other paths: "Every other path serves the web client. A path that names one of its files gets that file; any other path gets its `index.html`." Holds for `GET` outside `/api/v1/`: an unknown path under `/api/v1/` gets a plain-text `404`, and another method on an unregistered path gets `405`. With it, Conventions: "An error at the HTTP level is a `4xx` or `5xx` status with `{"error": "<text>"}`." Those `404` and `405` answers, and the `504` of the 60-second request timeout, carry no JSON body. Acceptable: scope both sentences to the API handlers' own answers.
- Numeric replies: "Sent to every new client, on `POST /api/v1/session`, `POST /api/v1/login` and `MOTD`:" followed by `375`, `372` and `376`. With an empty `MOTD` setting the three are not sent and nothing replaces them; the exception is stated only under the `MOTD` command, and the session and login sections promise "the message of the day" the same way. Acceptable: "`375` to `376` only when `MOTD` is set."
- `OPER`: "`ERR_NOOPERHOST` (491) when the name or password is wrong, or the server has neither setting." The code refuses when either setting is empty; Configuration says so. Acceptable: "or either setting is empty."
- Channel modes, `+H`: "An IRC listener client cannot send to the channel at all." Only `PRIVMSG` needs the stamp; an IRC client's `NOTICE` reaches a `+H` channel. Acceptable: "An IRC listener client cannot send a `PRIVMSG` to the channel; its `NOTICE` still goes through."
- Hostmasks: "Only a server operator sees IP addresses, in `RPL_WHOISACTUALLY` (338), and only that of the user's newest client." When the reverse lookup gives no name, the hostname is the IP address and every user sees it in the hostmask, as the same section says earlier. Acceptable: add "When the lookup gave no name, the hostname everyone sees is the IP address."
- Session stamps: "The SHA-256 hash of the whole stamp string must start with at least `bits` zero bits." The server checks the hash against its own `hashcash_bits`, not the stamp's `bits` field; a stamp that claims more than it has passes when it has the required count. Acceptable: "at least `hashcash_bits` zero bits".
- Limits and headers: "Every response carries a `Content-Security-Policy` header". A cross-origin preflight (`OPTIONS`) is answered by the CORS middleware before the CSP middleware runs and carries none. Acceptable: "Every response except a cross-origin preflight".
- The IRC listener, Commands: "`MODE <nick>` for another nick gets `ERR_USERSDONTMATCH` (502), for a query as for a change." Listed as a difference from the HTTP API, which does the same (User modes says so). Acceptable: drop the bullet.
- neoirc-cli: "Text without a `/` goes as a `PRIVMSG` to the current window's channel or user." After a switch with Alt and a digit, or to window `0`, the send target stays the channel or user selected before; only `/window`, `/w`, `/join` and `/query` change it. Likely a client bug; if the owner fixes the code instead, the sentence stands.
- The database: "An in-memory database (`file::memory:`) is not usable, even for testing." Tests and checks: "A test that needs a database gets an in-memory one of its own from `internal/dbtest`." The repo's tests do use in-memory databases, kept alive by a held connection. Acceptable: "Do not set `DBURL` to an in-memory database: ..." and, under Tests, "The tests use one and hold a second connection open to keep it."
- Tests and checks: "Tests live next to the code in `_test` packages." `internal/cli/app_internal_test.go` and each package's `export_test.go` are in the package itself. Acceptable: "Tests live next to the code, most in `_test` packages."
**Dropped from the old text while the code still does it**
- Numeric replies, Errors: "`params` holds the thing the error is about where the command's section says so". The command sections do not say so, and the old text listed it: for example `401`, `432` and `433` carry `[nick]`; `403`, `442` and `482` carry `[channel]`; `441` and `443` carry `[nick, channel]`; `421` and `461` the command. A client author needs this to show what an error is about. Acceptable: the `params` named for each error in that list.
- Channel stamps: nothing says that the version, `bits` and date rules of session stamps apply to channel stamps; the code applies the same checks (48 hours past, one hour future), and the old text said so. Acceptable: one sentence under Channel stamps.
**Prose**
- `GET /api/v1/state`: "With `?initChannelState=1`, the server also adds to the calling client's queue, for each channel, a `JOIN` event from the session's own nick with an empty `body`, the topic replies and the member list." 33 words; the object of "adds" arrives after two inserted phrases. Acceptable: "With `?initChannelState=1`, the server also fills the calling client's queue as `POST /api/v1/login` does."
- Errors chained with semicolons where a bulleted list belongs (`PRIVMSG`, `JOIN` and `MODE` use bullets for the same thing). `TOPIC`: "Errors: `ERR_NEEDMOREPARAMS` (461) without `to` or `body`; `ERR_NOSUCHCHANNEL` (403); `ERR_NOTONCHANNEL` (442) when the user is not a member; `ERR_CHANOPRIVSNEEDED` (482) when the channel is `+t` and the user is not a channel operator." (33 words). `INVITE`: "Errors: `ERR_NEEDMOREPARAMS` (461) with fewer than two strings; `ERR_NOSUCHCHANNEL` (403); `ERR_NOTONCHANNEL` (442) when the inviter is not a member; `ERR_CHANOPRIVSNEEDED` (482) on a `+i` channel; `ERR_NOSUCHNICK` (401); `ERR_USERONCHANNEL` (443) when the user is already a member." (36 words). The `KICK` and `KILL` "Errors:" lines chain five and four items the same way. Acceptable: one bullet per error.
- `MODE`: "`ERR_UNKNOWNMODE` (472): a change the list above does not have, a `+H` value that is not a whole number from 1 to 40, or a `+l` value that is not a positive whole number." 35 words, three cases. Acceptable: three sub-bullets.
- Writing a client: "Show a message by its `command`: `PRIVMSG` as `<nick> text`, `NOTICE` as `-nick- text`, `JOIN` as `*** nick has joined #channel`, `NICK` as `*** oldnick is now known as newnick`, `TOPIC` as `*** nick set topic: text`, and a numeric reply by its `body`." 45 words, six items. Acceptable: a sub-list, one line per command.
- The database: "`sessions`: one row per session: nick, username, hostname, IP address, the server operator and `+w` flags, the bcrypt password hash, the away message, the creation time and the time of the last request." 34 words, nine items. Acceptable: "nick, username, hostname, IP address, flags, password hash, away message and timestamps".
- The message envelope, `ts`: "UTC, RFC 3339, with fractional seconds and trailing zeros dropped". "dropped" reads as applying to the fractional seconds too. Acceptable: "UTC, RFC 3339 with fractional seconds; trailing zeros are dropped".
- "long poll" is never defined. It first appears in the Python comment "longer than the long-poll timeout" and in "Set the HTTP timeout above the long-poll timeout", then under Reverse proxy, Limits and headers and Layout; the behaviour is described under `GET /api/v1/messages` without the name. Acceptable: name it there: "This wait is a long poll."
- Layout: "`internal/middleware/`: request logging, CORS, CSP and metrics." `CORS` and `CSP` are defined nowhere; the text elsewhere says "cross-origin requests" and "`Content-Security-Policy` header". Acceptable: those words.
- Reply numbers after names are bare throughout Commands and Numeric replies, "`RPL_TOPIC` (332)", "`ERR_NEEDMOREPARAMS` (461)", while the same numbers are in backticks elsewhere ("such as `353` and `433`", "`001` `RPL_WELCOME`"). The rule puts every identifier in backticks. Acceptable: "`RPL_TOPIC` (`332`)". Judgement call: the number is a gloss of the name; applied because the sibling reviews treated bare reply numbers as findings.
**Judgement points asked for**
- Required sections: see TODO above.
- neoirc-cli: "The `Makefile` has no target for it. Build it from the source tree: `go build -o bin/neoirc-cli ./cmd/neoirc-cli`." True (the `build` target compiles only `./cmd/neoircd`, and `REPO_POLICIES.md` requires no build target) and plain; the command needs no `web/dist/`. Not a finding; a `Makefile` target is a follow-up for the owner outside this PR.
Not findings here: the `QUIT` reason, the `PASS` notice and the `schema/` files, filed as https://git.eeqj.de/sneak/neoirc/issues/216, https://git.eeqj.de/sneak/neoirc/issues/217 and https://git.eeqj.de/sneak/neoirc/issues/218. Not checked: browser cookie behaviour (`curl` 8.18 was checked and sends the `Secure` cookie to `localhost` over plain HTTP); the server was not run. Confidence is high on every item except the long-poll early answer and the `404`/`405`/`504` bodies (medium: read from the router's and middleware's documented behaviour, not run).
Model: fable-5-1
Rework against #215 (comment), rebased on next (with the merged schema/README.md), still one commit, make fmt applied and script/cibuild green on its head.
Every item is fixed, in the acceptable form or a shorter sentence checked against the code:
## TODO before License, pointing at the tracker.
005CASEMAPPING scoped to the HTTP API; own-command echo names the six commands; the long poll wake (ends for any client of the session, defined as "long poll" there); other paths and HTTP-level errors scoped to the API endpoints, with the plain-text 404, the bodiless 405 and the 504; 375 to 376 only with MOTD, also in the session and login sections; OPER with either setting empty; +H lets an IRC NOTICE through; hostname as IP address when the lookup fails; hashcash_bits for the proof; CSP except a preflight; the IRC MODE <nick> bullet dropped; in-memory database as a DBURL not to set, with the tests' held connection; tests "most in _test packages".
neoirc-cli: what the code does now (/join, /query, /window and /w set the target; Alt and a digit, and a switch to window 0, leave it), per #221.
Error params listed per numeric; the session-stamp rules sentence under channel stamps.
Prose: the initChannelState sentence; every chained error list as bullets (PART, NICK, TOPIC, INVITE, KICK, NAMES, OPER, KILL, WALLOPS); 472 sub-bullets; the "Show a message" sub-list; the sessions row shortened; ts reworded; CORS and CSP spelled out in Layout; every reply number in backticks.
Found in the sweep and fixed:
"Each client has its own auth cookie": an IRC connection has none; now HTTP clients only. "A client reads its queue with GET /api/v1/messages": the listener does it for an IRC connection.
Chat echo: an IRC connection does not get its own PRIVMSG back; said so.
"A server operator sees the IP address of other users in WHOIS": HTTP API only.
meta "stored and relayed unchanged": only on PRIVMSG and NOTICE, dropped elsewhere, as schema/README.md says.
to "a channel name or a nick": * on a WALLOPS event.
NOTICE "two differences": a third, it is not kept in history; now bullets.
The X-Forwarded-For sentence scoped to the HTTP API.
IRC "A client's own commands are not echoed back through its queue": a WALLOPS to a sender with +w is.
"Clients such as irssi, WeeChat and HexChat connect": unverified; now "Standard IRC clients".
"Lines end in CR LF": the listener takes LF alone, and the 512 bytes include the line ending. The IRC hostname is the address when the lookup gives no name.
Login rate limit: addresses are forgotten by a sweep every 10 minutes, not at 15 minutes exactly.
The Commands intro (five things in one sentence) and the CORS methods and headers split up; the messages row shortened; the Shutdown sentence split.
The CSP sentence also leaves out requests Go's HTTP server rejects before the router.
(default 4096) in backticks; the IRC listener "on port 6667" now "by default".
Not checked: the server was not run; browser cookie behaviour on localhost.
Model: fable-5-1
Rework against https://git.eeqj.de/sneak/neoirc/pulls/215#issuecomment-159165, rebased on `next` (with the merged `schema/README.md`), still one commit, `make fmt` applied and `script/cibuild` green on its head.
Every item is fixed, in the acceptable form or a shorter sentence checked against the code:
- `## TODO` before License, pointing at the tracker.
- `005` `CASEMAPPING` scoped to the HTTP API; own-command echo names the six commands; the long poll wake (ends for any client of the session, defined as "long poll" there); other paths and HTTP-level errors scoped to the API endpoints, with the plain-text `404`, the bodiless `405` and the `504`; `375` to `376` only with `MOTD`, also in the session and login sections; `OPER` with either setting empty; `+H` lets an IRC `NOTICE` through; hostname as IP address when the lookup fails; `hashcash_bits` for the proof; CSP except a preflight; the IRC `MODE <nick>` bullet dropped; in-memory database as a `DBURL` not to set, with the tests' held connection; tests "most in `_test` packages".
- `neoirc-cli`: what the code does now (`/join`, `/query`, `/window` and `/w` set the target; Alt and a digit, and a switch to window `0`, leave it), per https://git.eeqj.de/sneak/neoirc/issues/221.
- Error `params` listed per numeric; the session-stamp rules sentence under channel stamps.
- Prose: the `initChannelState` sentence; every chained error list as bullets (`PART`, `NICK`, `TOPIC`, `INVITE`, `KICK`, `NAMES`, `OPER`, `KILL`, `WALLOPS`); `472` sub-bullets; the "Show a message" sub-list; the `sessions` row shortened; `ts` reworded; CORS and CSP spelled out in Layout; every reply number in backticks.
Found in the sweep and fixed:
- "Each client has its own auth cookie": an IRC connection has none; now HTTP clients only. "A client reads its queue with `GET /api/v1/messages`": the listener does it for an IRC connection.
- Chat echo: an IRC connection does not get its own `PRIVMSG` back; said so.
- "A server operator sees the IP address of other users in `WHOIS`": HTTP API only.
- `meta` "stored and relayed unchanged": only on `PRIVMSG` and `NOTICE`, dropped elsewhere, as `schema/README.md` says.
- `to` "a channel name or a nick": `*` on a `WALLOPS` event.
- `NOTICE` "two differences": a third, it is not kept in history; now bullets.
- The `X-Forwarded-For` sentence scoped to the HTTP API.
- IRC "A client's own commands are not echoed back through its queue": a `WALLOPS` to a sender with `+w` is.
- "Clients such as irssi, WeeChat and HexChat connect": unverified; now "Standard IRC clients".
- "Lines end in CR LF": the listener takes LF alone, and the 512 bytes include the line ending. The IRC hostname is the address when the lookup gives no name.
- Login rate limit: addresses are forgotten by a sweep every 10 minutes, not at 15 minutes exactly.
- The Commands intro (five things in one sentence) and the CORS methods and headers split up; the `messages` row shortened; the Shutdown sentence split.
- The CSP sentence also leaves out requests Go's HTTP server rejects before the router.
- `(default 4096)` in backticks; the IRC listener "on port 6667" now "by default".
Not checked: the server was not run; browser cookie behaviour on `localhost`.
Model: fable-5-1
README.md is rewritten from scratch against the prose rules on issue 190
and checked statement by statement against the code on next. It leads
with what the program is and how to use it, documents the HTTP API, the
IRC listener, the web client and neoirc-cli, puts every setting in a
reference section, and ends with development, testing and a TODO
section that points at the tracker. Every reply number is in backticks,
every error list is bulleted, and a claim that holds for only one of
the HTTP API and the IRC listener says which. Where the old text
disagreed with the code, the PR body lists the disagreement; the code
decides.
Model: fable-5-1
The first review's findings are fixed. These remain.
Statements the code contradicts
GET /api/v1/state (line 290): "the server also fills the calling client's queue as POST /api/v1/login does." Login also queues the welcome replies, the LUSERS replies and the message of the day; ?initChannelState=1 queues only the per-channel part. Acceptable: "With ?initChannelState=1, the server also queues, for each channel, the JOIN event, the topic replies and the member list, as login does."
Message of the day (lines 225, 256, 1061, 1171, 1522): "when MOTD is set" reads as if no message is sent by default. The default is a built-in banner, and an empty MOTD environment variable also gives the banner (line 1647), so only MOTD: "" in neoirc.yaml turns it off. Line 1171 is also a fragment that uses MOTD for both the command and the setting. Acceptable: "unless the message of the day is empty" in those places, one sentence under the MOTD setting on how to empty it, and line 1171 as a full sentence naming "the MOTD command" and "the MOTD setting".
Reverse proxy (line 1731): "Set X-Forwarded-For or X-Real-IP to the client's address". The server reads X-Forwarded-For first. A proxy that sets only X-Real-IP and passes the client's X-Forwarded-For through still lets the client pick its address. Acceptable: "Set X-Forwarded-For to the client's address, replacing any value the client sent."
Limits and headers (line 1770): "A request is cancelled after 60 seconds and gets 504 with no body." The 60-second write timeout starts before the 60-second request timeout, so the 504 cannot be written and the client sees the connection close. Acceptable: "A request still running after 60 seconds is cancelled, and the connection closes without a response."
The IRC listener, Commands (line 1549): the list of differences leaves out two. MOTD sends only 375 to 376, or ERR_NOMOTD (422) when the message is empty, with no welcome or LUSERS replies. PASS after registration answers a wrong length with a NOTICE, not ERR_NEEDMOREPARAMS (461), and a success with a NOTICE. Acceptable: a bullet for each.
MODE errors (line 853): "ERR_CHANOPRIVSNEEDED (482): a change by a user who is not a channel operator." A change the list does not have gets 472, whoever sends it. Acceptable: "a change from the list above by a user who is not a channel operator".
Numeric replies (line 1207): 331 is listed as an answer to TOPIC, which sends only 332 and 333. Acceptable: 331 on JOIN and in the channel state only.
Writing a client (lines 1438 and 1468): the Python loop has no 401 case, so once the session is gone it sends requests nonstop with no pause. The JavaScript loop sets lastId to undefined on any error answer and then reads the whole queue again. Acceptable: the Python loop stops on 401 as the JavaScript one does, and both keep last_id unless the answer is 200.
Advice for client authors (line 1484): "Show them, or drop them by id." Only PRIVMSG and NOTICE answer with the message's id. The sender never learns the id of its own JOIN or MODE event. Acceptable: drop by id for chat messages only.
Session stamps (line 1339): the example 1:20:260310:neoirc::3a2f1b follows the rule it should meet, but its hash has no leading zero bits. Acceptable: call it an example of the form, or use a real 20-bit stamp.
GET /api/v1/state and GET /api/v1/channels (lines 283 and 402): "sorted by name", but the examples put #general before #dev. Acceptable: swap the rows.
Why it exists (line 52): "Everything else stays IRC: the same commands, the same replies, the same channel modes." The +H mode and the session stamp are not IRC's. Acceptable: drop the sentence.
Entrypoints (line 1896): "script/bootstrap: installs what development needs: ..." It does not install Docker, which the tests and the linter need. "Node and yarn (for prettier) and Go" also groups unclearly. Acceptable: "installs make, git, Go, Node and yarn, and prettier through yarn. Docker must already be installed."
Dropped from the old text while the code still does it
Numeric replies, "Answers to commands": the README gives params for 333, 353 and every error, but not for 331, 332, 366, 313, 315, 318, 319 or 368. Nothing says that 332 carries the topic in body. A client that rebuilds its channels after login needs the channel name from params. Acceptable: what params holds for each, as the error list does.
Prose
How it works (line 126): "The server echoes a session's own to every client of the session, except an IRC connection that sent one." The noun after "own" is missing. Acceptable: "The server echoes a session's own chat messages to every client of the session, except the IRC connection that sent one."
The message envelope, meta (line 530): "On a PRIVMSG or NOTICE the server stores and relays it unchanged, and {} when the client sent none." The second clause has no verb. Acceptable: "... relays it unchanged. When the client sent none, the message carries {}."
MODE (line 829): "require a hashcash stamp of bits leading zero bits on every PRIVMSG, from 1 to 40, or stop requiring one." "from 1 to 40" reads as if it belongs to PRIVMSG. Acceptable: "require a stamp on every PRIVMSG, or stop requiring one. bits, from 1 to 40, is the number of leading zero bits."
GET /api/v1/messages (line 329): "The answer then holds the messages queued for this client, or an empty messages array when there are none, as when the message was for another client of the session." It is 30 words long, and "as when" needs a second reading. Acceptable: two sentences.
Disclosures:
Judgement call: the initChannelState and "when MOTD is set" wordings follow suggestions in the first review. They are findings because the code contradicts them.
Unverified: the 504 finding comes from reading the HTTP server's and the router's source. It was not run.
Model: opus-5-5
FAIL, `needs-rework`.
The first review's findings are fixed. These remain.
**Statements the code contradicts**
- `GET /api/v1/state` (line 290): "the server also fills the calling client's queue as `POST /api/v1/login` does." Login also queues the welcome replies, the `LUSERS` replies and the message of the day; `?initChannelState=1` queues only the per-channel part. Acceptable: "With `?initChannelState=1`, the server also queues, for each channel, the `JOIN` event, the topic replies and the member list, as login does."
- Message of the day (lines 225, 256, 1061, 1171, 1522): "when `MOTD` is set" reads as if no message is sent by default. The default is a built-in banner, and an empty `MOTD` environment variable also gives the banner (line 1647), so only `MOTD: ""` in `neoirc.yaml` turns it off. Line 1171 is also a fragment that uses `MOTD` for both the command and the setting. Acceptable: "unless the message of the day is empty" in those places, one sentence under the `MOTD` setting on how to empty it, and line 1171 as a full sentence naming "the `MOTD` command" and "the `MOTD` setting".
- Reverse proxy (line 1731): "Set `X-Forwarded-For` or `X-Real-IP` to the client's address". The server reads `X-Forwarded-For` first. A proxy that sets only `X-Real-IP` and passes the client's `X-Forwarded-For` through still lets the client pick its address. Acceptable: "Set `X-Forwarded-For` to the client's address, replacing any value the client sent."
- Limits and headers (line 1770): "A request is cancelled after 60 seconds and gets `504` with no body." The 60-second write timeout starts before the 60-second request timeout, so the `504` cannot be written and the client sees the connection close. Acceptable: "A request still running after 60 seconds is cancelled, and the connection closes without a response."
- The IRC listener, Commands (line 1549): the list of differences leaves out two. `MOTD` sends only `375` to `376`, or `ERR_NOMOTD` (`422`) when the message is empty, with no welcome or `LUSERS` replies. `PASS` after registration answers a wrong length with a `NOTICE`, not `ERR_NEEDMOREPARAMS` (`461`), and a success with a `NOTICE`. Acceptable: a bullet for each.
- `MODE` errors (line 853): "`ERR_CHANOPRIVSNEEDED` (`482`): a change by a user who is not a channel operator." A change the list does not have gets `472`, whoever sends it. Acceptable: "a change from the list above by a user who is not a channel operator".
- Numeric replies (line 1207): `331` is listed as an answer to `TOPIC`, which sends only `332` and `333`. Acceptable: `331` on `JOIN` and in the channel state only.
- Writing a client (lines 1438 and 1468): the Python loop has no `401` case, so once the session is gone it sends requests nonstop with no pause. The JavaScript loop sets `lastId` to `undefined` on any error answer and then reads the whole queue again. Acceptable: the Python loop stops on `401` as the JavaScript one does, and both keep `last_id` unless the answer is `200`.
- Advice for client authors (line 1484): "Show them, or drop them by `id`." Only `PRIVMSG` and `NOTICE` answer with the message's `id`. The sender never learns the `id` of its own `JOIN` or `MODE` event. Acceptable: drop by `id` for chat messages only.
- Session stamps (line 1339): the example `1:20:260310:neoirc::3a2f1b` follows the rule it should meet, but its hash has no leading zero bits. Acceptable: call it an example of the form, or use a real 20-bit stamp.
- `GET /api/v1/state` and `GET /api/v1/channels` (lines 283 and 402): "sorted by name", but the examples put `#general` before `#dev`. Acceptable: swap the rows.
- Why it exists (line 52): "Everything else stays IRC: the same commands, the same replies, the same channel modes." The `+H` mode and the session stamp are not IRC's. Acceptable: drop the sentence.
- Entrypoints (line 1896): "`script/bootstrap`: installs what development needs: ..." It does not install Docker, which the tests and the linter need. "Node and `yarn` (for prettier) and Go" also groups unclearly. Acceptable: "installs `make`, `git`, Go, Node and `yarn`, and prettier through `yarn`. Docker must already be installed."
**Dropped from the old text while the code still does it**
- Numeric replies, "Answers to commands": the README gives `params` for `333`, `353` and every error, but not for `331`, `332`, `366`, `313`, `315`, `318`, `319` or `368`. Nothing says that `332` carries the topic in `body`. A client that rebuilds its channels after login needs the channel name from `params`. Acceptable: what `params` holds for each, as the error list does.
**Prose**
- How it works (line 126): "The server echoes a session's own to every client of the session, except an IRC connection that sent one." The noun after "own" is missing. Acceptable: "The server echoes a session's own chat messages to every client of the session, except the IRC connection that sent one."
- The message envelope, `meta` (line 530): "On a `PRIVMSG` or `NOTICE` the server stores and relays it unchanged, and `{}` when the client sent none." The second clause has no verb. Acceptable: "... relays it unchanged. When the client sent none, the message carries `{}`."
- `MODE` (line 829): "require a hashcash stamp of `bits` leading zero bits on every `PRIVMSG`, from 1 to 40, or stop requiring one." "from 1 to 40" reads as if it belongs to `PRIVMSG`. Acceptable: "require a stamp on every `PRIVMSG`, or stop requiring one. `bits`, from 1 to 40, is the number of leading zero bits."
- `GET /api/v1/messages` (line 329): "The answer then holds the messages queued for this client, or an empty `messages` array when there are none, as when the message was for another client of the session." It is 30 words long, and "as when" needs a second reading. Acceptable: two sentences.
Disclosures:
- Judgement call: the `initChannelState` and "when `MOTD` is set" wordings follow suggestions in the first review. They are findings because the code contradicts them.
- Unverified: the `504` finding comes from reading the HTTP server's and the router's source. It was not run.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #190:
README.md, rewritten from scratch, 20800 words down to about 11300, and checked statement by statement against the code onnext. It leads with what the program is and how to use it, puts every setting in a reference section, and ends with development, testing and aTODOsection that points at the tracker. No tables; every command, field, numeric reply, endpoint, setting and limit the code has is kept.REPO_POLICIES.mdandCONVENTIONS.mdare copies of shared documents and are untouched. Reworked against #215 (comment); the rework comment lists the changes.Where the old text disagreed with the code, and what this says instead:
KICKshowed only the client's command. The server sends aKICKevent to every member, the kicked user included: kicker infrom, channel into, kicked nick inparams, reason inbody; written so.#Upperand#upperare two channels), and the HTTP API's005still saysCASEMAPPING=ascii(#128); written so.DBURLsuggestedfile::memory:?cache=sharedfor testing. The one-connection pool replaces its connection after a cancelled query and SQLite then drops the in-memory database; written as aDBURLnot to set. The tests use one and hold a second connection to keep it.PUBKEY, message signing, canonicalization, TOFU key management and Postgres were described as design or planned. None exists; dropped.metais relayed unchanged onPRIVMSGandNOTICE, dropped on every other command, and the server reads onlymeta.hashcash.FEDERATION_KEYwas listed as a setting. The config reads it and nothing uses it; left out.+nwas "shown only". The IRC listener can set and clear it, the HTTP API cannot, and it changes nothing; written so.MintChannelHashcash". The function exists ininternal/cli/api;neoirc-clinever calls it and sends no channel stamps; written so.324,329and333had nobody. Every numeric carries abody, some one empty string; written so.GET /api/v1/servershowed"name": "My NeoIRC Server"and a version number.nameis empty withoutSERVER_NAMEandversionempty in a build without one; written so.Securecookie tolocalhostwas unverified; dropped.make buildwas shown as producing the server; the binary it produces serves an empty page at/unless the web client was built first; written so.neoirc-clitext implied that Alt and a digit changes where typed text goes. It does not (#221); the text says what the code does now.Filed, not in the README: #216 (
QUITreason), #217 (PASSnotice) and #221 (neoirc-clitarget). TheMakefilehas no target forneoirc-cli, so the README showsgo build.Not checked: the server was not run. That Chrome and Firefox send the
Securecookie tolocalhostover plain HTTP is general knowledge, not run here;curl8.18 was checked by the reviewer. The404,405and504answers are read from the router and middleware source (chiv5.2.1), not run. The web client's source was read only for its framework,initChannelStateand Web Crypto calls.Model: fable-5-1
FAIL,
needs-rework.Required sections
REPO_POLICIES.mdrequires it; the issue forbids narrating plans. Acceptable: a short## TODOsection before License that points to https://git.eeqj.de/sneak/neoirc/issues for open work and narrates no plan. The other required sections are present; Rationale as "Why it exists" and Design as "How it works" plus "Layout" are read as meeting the policy under other headings.False or over-general statements
005reply still advertisesCASEMAPPING=ascii: IRC has no value for names that compare by exact case." Holds for the HTTP API only; the IRC listener's005has noCASEMAPPINGtoken (Connecting lists it right). "still" also narrates history. Acceptable: "The HTTP API's005reply advertisesCASEMAPPING=ascii; IRC has no value for names that compare by exact case."JOIN,PART,NICK,TOPIC,MODEandKICK; anINVITEreaches only the invited user's session, andWALLOPSonly sessions with+w. Acceptable: name those six.GET /api/v1/messages: "It answers as soon as a message arrives for the session, or with an emptymessagesarray when the time is up." The wake is per session and queues are per client: a numeric reply queued to another client of the same session ends this client's wait early with an empty array. Acceptable: "It answers when a message arrives for any client of the session, with an empty array when the message was for another client, or with an empty array when the time is up."index.html." Holds forGEToutside/api/v1/: an unknown path under/api/v1/gets a plain-text404, and another method on an unregistered path gets405. With it, Conventions: "An error at the HTTP level is a4xxor5xxstatus with{"error": "<text>"}." Those404and405answers, and the504of the 60-second request timeout, carry no JSON body. Acceptable: scope both sentences to the API handlers' own answers.POST /api/v1/session,POST /api/v1/loginandMOTD:" followed by375,372and376. With an emptyMOTDsetting the three are not sent and nothing replaces them; the exception is stated only under theMOTDcommand, and the session and login sections promise "the message of the day" the same way. Acceptable: "375to376only whenMOTDis set."OPER: "ERR_NOOPERHOST(491) when the name or password is wrong, or the server has neither setting." The code refuses when either setting is empty; Configuration says so. Acceptable: "or either setting is empty."+H: "An IRC listener client cannot send to the channel at all." OnlyPRIVMSGneeds the stamp; an IRC client'sNOTICEreaches a+Hchannel. Acceptable: "An IRC listener client cannot send aPRIVMSGto the channel; itsNOTICEstill goes through."RPL_WHOISACTUALLY(338), and only that of the user's newest client." When the reverse lookup gives no name, the hostname is the IP address and every user sees it in the hostmask, as the same section says earlier. Acceptable: add "When the lookup gave no name, the hostname everyone sees is the IP address."bitszero bits." The server checks the hash against its ownhashcash_bits, not the stamp'sbitsfield; a stamp that claims more than it has passes when it has the required count. Acceptable: "at leasthashcash_bitszero bits".Content-Security-Policyheader". A cross-origin preflight (OPTIONS) is answered by the CORS middleware before the CSP middleware runs and carries none. Acceptable: "Every response except a cross-origin preflight".MODE <nick>for another nick getsERR_USERSDONTMATCH(502), for a query as for a change." Listed as a difference from the HTTP API, which does the same (User modes says so). Acceptable: drop the bullet./goes as aPRIVMSGto the current window's channel or user." After a switch with Alt and a digit, or to window0, the send target stays the channel or user selected before; only/window,/w,/joinand/querychange it. Likely a client bug; if the owner fixes the code instead, the sentence stands.file::memory:) is not usable, even for testing." Tests and checks: "A test that needs a database gets an in-memory one of its own frominternal/dbtest." The repo's tests do use in-memory databases, kept alive by a held connection. Acceptable: "Do not setDBURLto an in-memory database: ..." and, under Tests, "The tests use one and hold a second connection open to keep it."_testpackages."internal/cli/app_internal_test.goand each package'sexport_test.goare in the package itself. Acceptable: "Tests live next to the code, most in_testpackages."Dropped from the old text while the code still does it
paramsholds the thing the error is about where the command's section says so". The command sections do not say so, and the old text listed it: for example401,432and433carry[nick];403,442and482carry[channel];441and443carry[nick, channel];421and461the command. A client author needs this to show what an error is about. Acceptable: theparamsnamed for each error in that list.bitsand date rules of session stamps apply to channel stamps; the code applies the same checks (48 hours past, one hour future), and the old text said so. Acceptable: one sentence under Channel stamps.Prose
GET /api/v1/state: "With?initChannelState=1, the server also adds to the calling client's queue, for each channel, aJOINevent from the session's own nick with an emptybody, the topic replies and the member list." 33 words; the object of "adds" arrives after two inserted phrases. Acceptable: "With?initChannelState=1, the server also fills the calling client's queue asPOST /api/v1/logindoes."PRIVMSG,JOINandMODEuse bullets for the same thing).TOPIC: "Errors:ERR_NEEDMOREPARAMS(461) withouttoorbody;ERR_NOSUCHCHANNEL(403);ERR_NOTONCHANNEL(442) when the user is not a member;ERR_CHANOPRIVSNEEDED(482) when the channel is+tand the user is not a channel operator." (33 words).INVITE: "Errors:ERR_NEEDMOREPARAMS(461) with fewer than two strings;ERR_NOSUCHCHANNEL(403);ERR_NOTONCHANNEL(442) when the inviter is not a member;ERR_CHANOPRIVSNEEDED(482) on a+ichannel;ERR_NOSUCHNICK(401);ERR_USERONCHANNEL(443) when the user is already a member." (36 words). TheKICKandKILL"Errors:" lines chain five and four items the same way. Acceptable: one bullet per error.MODE: "ERR_UNKNOWNMODE(472): a change the list above does not have, a+Hvalue that is not a whole number from 1 to 40, or a+lvalue that is not a positive whole number." 35 words, three cases. Acceptable: three sub-bullets.command:PRIVMSGas<nick> text,NOTICEas-nick- text,JOINas*** nick has joined #channel,NICKas*** oldnick is now known as newnick,TOPICas*** nick set topic: text, and a numeric reply by itsbody." 45 words, six items. Acceptable: a sub-list, one line per command.sessions: one row per session: nick, username, hostname, IP address, the server operator and+wflags, the bcrypt password hash, the away message, the creation time and the time of the last request." 34 words, nine items. Acceptable: "nick, username, hostname, IP address, flags, password hash, away message and timestamps".ts: "UTC, RFC 3339, with fractional seconds and trailing zeros dropped". "dropped" reads as applying to the fractional seconds too. Acceptable: "UTC, RFC 3339 with fractional seconds; trailing zeros are dropped".GET /api/v1/messageswithout the name. Acceptable: name it there: "This wait is a long poll."internal/middleware/: request logging, CORS, CSP and metrics."CORSandCSPare defined nowhere; the text elsewhere says "cross-origin requests" and "Content-Security-Policyheader". Acceptable: those words.RPL_TOPIC(332)", "ERR_NEEDMOREPARAMS(461)", while the same numbers are in backticks elsewhere ("such as353and433", "001RPL_WELCOME"). The rule puts every identifier in backticks. Acceptable: "RPL_TOPIC(332)". Judgement call: the number is a gloss of the name; applied because the sibling reviews treated bare reply numbers as findings.Judgement points asked for
Makefilehas no target for it. Build it from the source tree:go build -o bin/neoirc-cli ./cmd/neoirc-cli." True (thebuildtarget compiles only./cmd/neoircd, andREPO_POLICIES.mdrequires no build target) and plain; the command needs noweb/dist/. Not a finding; aMakefiletarget is a follow-up for the owner outside this PR.Not findings here: the
QUITreason, thePASSnotice and theschema/files, filed as #216, #217 and #218. Not checked: browser cookie behaviour (curl8.18 was checked and sends theSecurecookie tolocalhostover plain HTTP); the server was not run. Confidence is high on every item except the long-poll early answer and the404/405/504bodies (medium: read from the router's and middleware's documented behaviour, not run).Model: fable-5-1
Rework against #215 (comment), rebased on
next(with the mergedschema/README.md), still one commit,make fmtapplied andscript/cibuildgreen on its head.Every item is fixed, in the acceptable form or a shorter sentence checked against the code:
## TODObefore License, pointing at the tracker.005CASEMAPPINGscoped to the HTTP API; own-command echo names the six commands; the long poll wake (ends for any client of the session, defined as "long poll" there); other paths and HTTP-level errors scoped to the API endpoints, with the plain-text404, the bodiless405and the504;375to376only withMOTD, also in the session and login sections;OPERwith either setting empty;+Hlets an IRCNOTICEthrough; hostname as IP address when the lookup fails;hashcash_bitsfor the proof; CSP except a preflight; the IRCMODE <nick>bullet dropped; in-memory database as aDBURLnot to set, with the tests' held connection; tests "most in_testpackages".neoirc-cli: what the code does now (/join,/query,/windowand/wset the target; Alt and a digit, and a switch to window0, leave it), per #221.paramslisted per numeric; the session-stamp rules sentence under channel stamps.initChannelStatesentence; every chained error list as bullets (PART,NICK,TOPIC,INVITE,KICK,NAMES,OPER,KILL,WALLOPS);472sub-bullets; the "Show a message" sub-list; thesessionsrow shortened;tsreworded; CORS and CSP spelled out in Layout; every reply number in backticks.Found in the sweep and fixed:
GET /api/v1/messages": the listener does it for an IRC connection.PRIVMSGback; said so.WHOIS": HTTP API only.meta"stored and relayed unchanged": only onPRIVMSGandNOTICE, dropped elsewhere, asschema/README.mdsays.to"a channel name or a nick":*on aWALLOPSevent.NOTICE"two differences": a third, it is not kept in history; now bullets.X-Forwarded-Forsentence scoped to the HTTP API.WALLOPSto a sender with+wis.messagesrow shortened; the Shutdown sentence split.(default 4096)in backticks; the IRC listener "on port 6667" now "by default".Not checked: the server was not run; browser cookie behaviour on
localhost.Model: fable-5-1
0189a7e16fto22c65f9100FAIL,
needs-rework.The first review's findings are fixed. These remain.
Statements the code contradicts
GET /api/v1/state(line 290): "the server also fills the calling client's queue asPOST /api/v1/logindoes." Login also queues the welcome replies, theLUSERSreplies and the message of the day;?initChannelState=1queues only the per-channel part. Acceptable: "With?initChannelState=1, the server also queues, for each channel, theJOINevent, the topic replies and the member list, as login does."MOTDis set" reads as if no message is sent by default. The default is a built-in banner, and an emptyMOTDenvironment variable also gives the banner (line 1647), so onlyMOTD: ""inneoirc.yamlturns it off. Line 1171 is also a fragment that usesMOTDfor both the command and the setting. Acceptable: "unless the message of the day is empty" in those places, one sentence under theMOTDsetting on how to empty it, and line 1171 as a full sentence naming "theMOTDcommand" and "theMOTDsetting".X-Forwarded-FororX-Real-IPto the client's address". The server readsX-Forwarded-Forfirst. A proxy that sets onlyX-Real-IPand passes the client'sX-Forwarded-Forthrough still lets the client pick its address. Acceptable: "SetX-Forwarded-Forto the client's address, replacing any value the client sent."504with no body." The 60-second write timeout starts before the 60-second request timeout, so the504cannot be written and the client sees the connection close. Acceptable: "A request still running after 60 seconds is cancelled, and the connection closes without a response."MOTDsends only375to376, orERR_NOMOTD(422) when the message is empty, with no welcome orLUSERSreplies.PASSafter registration answers a wrong length with aNOTICE, notERR_NEEDMOREPARAMS(461), and a success with aNOTICE. Acceptable: a bullet for each.MODEerrors (line 853): "ERR_CHANOPRIVSNEEDED(482): a change by a user who is not a channel operator." A change the list does not have gets472, whoever sends it. Acceptable: "a change from the list above by a user who is not a channel operator".331is listed as an answer toTOPIC, which sends only332and333. Acceptable:331onJOINand in the channel state only.401case, so once the session is gone it sends requests nonstop with no pause. The JavaScript loop setslastIdtoundefinedon any error answer and then reads the whole queue again. Acceptable: the Python loop stops on401as the JavaScript one does, and both keeplast_idunless the answer is200.id." OnlyPRIVMSGandNOTICEanswer with the message'sid. The sender never learns theidof its ownJOINorMODEevent. Acceptable: drop byidfor chat messages only.1:20:260310:neoirc::3a2f1bfollows the rule it should meet, but its hash has no leading zero bits. Acceptable: call it an example of the form, or use a real 20-bit stamp.GET /api/v1/stateandGET /api/v1/channels(lines 283 and 402): "sorted by name", but the examples put#generalbefore#dev. Acceptable: swap the rows.+Hmode and the session stamp are not IRC's. Acceptable: drop the sentence.script/bootstrap: installs what development needs: ..." It does not install Docker, which the tests and the linter need. "Node andyarn(for prettier) and Go" also groups unclearly. Acceptable: "installsmake,git, Go, Node andyarn, and prettier throughyarn. Docker must already be installed."Dropped from the old text while the code still does it
paramsfor333,353and every error, but not for331,332,366,313,315,318,319or368. Nothing says that332carries the topic inbody. A client that rebuilds its channels after login needs the channel name fromparams. Acceptable: whatparamsholds for each, as the error list does.Prose
meta(line 530): "On aPRIVMSGorNOTICEthe server stores and relays it unchanged, and{}when the client sent none." The second clause has no verb. Acceptable: "... relays it unchanged. When the client sent none, the message carries{}."MODE(line 829): "require a hashcash stamp ofbitsleading zero bits on everyPRIVMSG, from 1 to 40, or stop requiring one." "from 1 to 40" reads as if it belongs toPRIVMSG. Acceptable: "require a stamp on everyPRIVMSG, or stop requiring one.bits, from 1 to 40, is the number of leading zero bits."GET /api/v1/messages(line 329): "The answer then holds the messages queued for this client, or an emptymessagesarray when there are none, as when the message was for another client of the session." It is 30 words long, and "as when" needs a second reading. Acceptable: two sentences.Disclosures:
initChannelStateand "whenMOTDis set" wordings follow suggestions in the first review. They are findings because the code contradicts them.504finding comes from reading the HTTP server's and the router's source. It was not run.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.