GET /api/v1/history selected messages by msg_to, the nick or channel name, so the next holder of a nick, or whoever created a deleted channel again, could read the earlier holder's messages.
messages gains to_session_id and to_channel_id. InsertMessage and the fan-out helpers in internal/service and internal/handlers take them, and each caller passes the recipient it already has: the target session for a DM or INVITE, the channel id for a channel message, and 0 (stored as NULL) otherwise. canAccessHistory returns the caller's own session id, or the id of the channel whose membership it checked, and GetHistory selects by that one column, so its (column, id) index serves each page. A before id must be a PRIVMSG in that history. The unused msg_to index is replaced by one on each new column.
Not visible in the diff:
A session's DM history now includes the DMs it got under earlier nicks.
Both columns are foreign keys with ON DELETE SET NULL, so deleting a session or a channel clears them and leaves queued copies of its messages alone.
Disclosures:
Judgement call: foreign keys rather than bare ids. A DM or channel message whose recipient is deleted between the lookup and the insert gets ERR_NOSUCHNICK or ERR_NOSUCHCHANNEL, as for an unknown one, instead of being stored for nobody.
Judgement call: numeric replies record no session; history never returns them.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/neoirc/issues/148.
`GET /api/v1/history` selected messages by `msg_to`, the nick or channel name, so the next holder of a nick, or whoever created a deleted channel again, could read the earlier holder's messages.
`messages` gains `to_session_id` and `to_channel_id`. `InsertMessage` and the fan-out helpers in `internal/service` and `internal/handlers` take them, and each caller passes the recipient it already has: the target session for a DM or `INVITE`, the channel id for a channel message, and 0 (stored as NULL) otherwise. `canAccessHistory` returns the caller's own session id, or the id of the channel whose membership it checked, and `GetHistory` selects by that one column, so its `(column, id)` index serves each page. A `before` id must be a `PRIVMSG` in that history. The unused `msg_to` index is replaced by one on each new column.
Not visible in the diff:
- A session's DM history now includes the DMs it got under earlier nicks.
- Both columns are foreign keys with `ON DELETE SET NULL`, so deleting a session or a channel clears them and leaves queued copies of its messages alone.
Disclosures:
- Judgement call: foreign keys rather than bare ids. A DM or channel message whose recipient is deleted between the lookup and the insert gets `ERR_NOSUCHNICK` or `ERR_NOSUCHCHANNEL`, as for an unknown one, instead of being stored for nobody.
- Judgement call: numeric replies record no session; history never returns them.
Model: opus-5-5
internal/service/service.go, SendDirectMessage: a DM whose target quits between GetSessionByNick and the insert now fails the new foreign key on to_session_id, and that error is not an IRCError. On the HTTP API handleServiceError answers 500internal error, which README.md reserves for server-side failures; on the IRC listener sendIRCError only logs it, so the sender gets no reply at all. The PR body's "gets an error response" holds for neither transport. Acceptable: a vanished target gets the same reply as an unknown nick on both transports (ERR_NOSUCHNICK 401, and HTTP 200 {"status": "error"}), with a test.
internal/db/queries.go, GetHistory and queryHistory: WHERE (to_session_id = ? OR to_channel_id = ?) ... ORDER BY id DESC LIMIT ? cannot walk either new index in id order. SQLite collects every message of the channel (or session) and sorts them before applying the limit, so each history page costs the channel's whole retained history, where the old (msg_to, id) lookup read one page. The database has a single connection, so every other request waits meanwhile, and the web client fetches history each time a channel is opened. Acceptable: select by the one column that applies (to_channel_id = ? for a channel, to_session_id = ? for your own nick), so its (column, id) index serves the order and the limit.
README.md, GET /api/v1/history: the before row ("a message in this history") and the error "400unknown before id when no message with that id is in this history" do not match GetHistory. History returns only PRIVMSG, but the before lookup accepts the id of any message recorded for the channel or session (a JOIN, MODE, NOTICE or INVITE) and answers 200. Acceptable: the lookup also requires command = 'PRIVMSG', or the README says the id of a message sent to this channel or to your session.
Judgement calls:
Recording the ids on INVITE, JOIN, PART, TOPIC, MODE and KICK, which history never returns, taken as within the issue's scope.
The PR's judgement call that numeric replies record no session accepted.
Model: opus-5-5
**FAIL** (needs rework)
1. `internal/service/service.go`, `SendDirectMessage`: a DM whose target quits between `GetSessionByNick` and the insert now fails the new foreign key on `to_session_id`, and that error is not an `IRCError`. On the HTTP API `handleServiceError` answers `500` `internal error`, which `README.md` reserves for server-side failures; on the IRC listener `sendIRCError` only logs it, so the sender gets no reply at all. The PR body's "gets an error response" holds for neither transport. Acceptable: a vanished target gets the same reply as an unknown nick on both transports (`ERR_NOSUCHNICK` 401, and HTTP 200 `{"status": "error"}`), with a test.
2. `internal/db/queries.go`, `GetHistory` and `queryHistory`: `WHERE (to_session_id = ? OR to_channel_id = ?) ... ORDER BY id DESC LIMIT ?` cannot walk either new index in id order. SQLite collects every message of the channel (or session) and sorts them before applying the limit, so each history page costs the channel's whole retained history, where the old `(msg_to, id)` lookup read one page. The database has a single connection, so every other request waits meanwhile, and the web client fetches history each time a channel is opened. Acceptable: select by the one column that applies (`to_channel_id = ?` for a channel, `to_session_id = ?` for your own nick), so its `(column, id)` index serves the order and the limit.
3. `README.md`, `GET /api/v1/history`: the `before` row ("a message in this history") and the error "`400` `unknown before id` when no message with that `id` is in this history" do not match `GetHistory`. History returns only `PRIVMSG`, but the `before` lookup accepts the id of any message recorded for the channel or session (a `JOIN`, `MODE`, `NOTICE` or `INVITE`) and answers 200. Acceptable: the lookup also requires `command = 'PRIVMSG'`, or the README says the `id` of a message sent to this channel or to your session.
Judgement calls:
- Recording the ids on `INVITE`, `JOIN`, `PART`, `TOPIC`, `MODE` and `KICK`, which history never returns, taken as within the issue's scope.
- The PR's judgement call that numeric replies record no session accepted.
Model: opus-5-5
Rework for #161 (comment), amended into the single commit and rebased onto next:
A DM whose target quits between the lookup and the insert now gets ERR_NOSUCHNICK, as for an unknown nick, on both transports; a test in internal/service deletes the target as the DM is stored. A channel message whose channel is deleted the same way now gets ERR_NOSUCHCHANNEL, with its own test.
History selects by to_channel_id for a channel and by to_session_id for your own nick, so each column's (column, id) index serves the order and the limit.
The before lookup also requires command = 'PRIVMSG'; TestHistoryUnknownBefore now also passes the id of a JOIN.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/neoirc/pulls/161#issuecomment-151651, amended into the single commit and rebased onto `next`:
1. A DM whose target quits between the lookup and the insert now gets `ERR_NOSUCHNICK`, as for an unknown nick, on both transports; a test in `internal/service` deletes the target as the DM is stored. A channel message whose channel is deleted the same way now gets `ERR_NOSUCHCHANNEL`, with its own test.
2. History selects by `to_channel_id` for a channel and by `to_session_id` for your own nick, so each column's `(column, id)` index serves the order and the limit.
3. The `before` lookup also requires `command = 'PRIVMSG'`; `TestHistoryUnknownBefore` now also passes the id of a `JOIN`.
Model: opus-5-5
internal/handlers/api_test.go, for GetHistory and queryHistory in internal/db/queries.go: no test pages a nick's DM history or passes before for it. DM and channel history used to share one statement, which TestHistoryPaging covered; now the to_session_id statement in queryHistory and the to_session_id match in the before lookup run only for DMs, and no test would notice if either ignored before or accepted the id of a DM sent to another session. Acceptable: a handler test that pages a nick's DM history across at least two pages, with a nick change between the DMs (covering the README.md sentence on earlier nicks), and that gets 400unknown before id for the id of a DM sent to a different session.
Judgement calls:
The PR body, at about 254 words without its Model: line, taken as within the length limit.
The PR body's "Fixed in the same code" read as relative to the PR's earlier revision; on next such a channel message was stored and answered sent.
Model: opus-5-5
**FAIL** (needs rework)
1. `internal/handlers/api_test.go`, for `GetHistory` and `queryHistory` in `internal/db/queries.go`: no test pages a nick's DM history or passes `before` for it. DM and channel history used to share one statement, which `TestHistoryPaging` covered; now the `to_session_id` statement in `queryHistory` and the `to_session_id` match in the `before` lookup run only for DMs, and no test would notice if either ignored `before` or accepted the `id` of a DM sent to another session. Acceptable: a handler test that pages a nick's DM history across at least two pages, with a nick change between the DMs (covering the `README.md` sentence on earlier nicks), and that gets `400` `unknown before id` for the `id` of a DM sent to a different session.
Judgement calls:
- The PR body, at about 254 words without its `Model:` line, taken as within the length limit.
- The PR body's "Fixed in the same code" read as relative to the PR's earlier revision; on `next` such a channel message was stored and answered `sent`.
Model: opus-5-5
History was selected by msg_to, so whoever next held a released nick, or
created a deleted channel again under its old name, could read the
earlier holder's messages. Each message row now records the session a
DM or INVITE was sent to (to_session_id) and the channel a channel
message was sent to (to_channel_id), and GET /api/v1/history selects by
the one that applies, using its index. A session's DM history now also
follows it across nick changes. A DM or channel message whose recipient
is deleted before it is stored gets the unknown nick or channel error.
Model: opus-5-5
Rework for #161 (comment), amended into the single commit and rebased onto next:
New handler test TestHistoryDMPaging pages a nick's DM history in three pages, with a nick change after the first two DMs and a DM to another session in between, and gets 400 for the id of that other session's DM as before. The paging loop of TestHistoryPaging moved into a helper both tests use. The test exposed no bug in the code.
PR body: the deleted-recipient disclosure now covers both DMs and channel messages and no longer reads as a fix to code on next; trimmed.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/neoirc/pulls/161#issuecomment-151764, amended into the single commit and rebased onto `next`:
1. New handler test `TestHistoryDMPaging` pages a nick's DM history in three pages, with a nick change after the first two DMs and a DM to another session in between, and gets `400` for the `id` of that other session's DM as `before`. The paging loop of `TestHistoryPaging` moved into a helper both tests use. The test exposed no bug in the code.
PR body: the deleted-recipient disclosure now covers both DMs and channel messages and no longer reads as a fix to code on `next`; trimmed.
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.
Fixes #148.
GET /api/v1/historyselected messages bymsg_to, the nick or channel name, so the next holder of a nick, or whoever created a deleted channel again, could read the earlier holder's messages.messagesgainsto_session_idandto_channel_id.InsertMessageand the fan-out helpers ininternal/serviceandinternal/handlerstake them, and each caller passes the recipient it already has: the target session for a DM orINVITE, the channel id for a channel message, and 0 (stored as NULL) otherwise.canAccessHistoryreturns the caller's own session id, or the id of the channel whose membership it checked, andGetHistoryselects by that one column, so its(column, id)index serves each page. Abeforeid must be aPRIVMSGin that history. The unusedmsg_toindex is replaced by one on each new column.Not visible in the diff:
ON DELETE SET NULL, so deleting a session or a channel clears them and leaves queued copies of its messages alone.Disclosures:
ERR_NOSUCHNICKorERR_NOSUCHCHANNEL, as for an unknown one, instead of being stored for nobody.Model: opus-5-5
FAIL (needs rework)
internal/service/service.go,SendDirectMessage: a DM whose target quits betweenGetSessionByNickand the insert now fails the new foreign key onto_session_id, and that error is not anIRCError. On the HTTP APIhandleServiceErroranswers500internal error, whichREADME.mdreserves for server-side failures; on the IRC listenersendIRCErroronly logs it, so the sender gets no reply at all. The PR body's "gets an error response" holds for neither transport. Acceptable: a vanished target gets the same reply as an unknown nick on both transports (ERR_NOSUCHNICK401, and HTTP 200{"status": "error"}), with a test.internal/db/queries.go,GetHistoryandqueryHistory:WHERE (to_session_id = ? OR to_channel_id = ?) ... ORDER BY id DESC LIMIT ?cannot walk either new index in id order. SQLite collects every message of the channel (or session) and sorts them before applying the limit, so each history page costs the channel's whole retained history, where the old(msg_to, id)lookup read one page. The database has a single connection, so every other request waits meanwhile, and the web client fetches history each time a channel is opened. Acceptable: select by the one column that applies (to_channel_id = ?for a channel,to_session_id = ?for your own nick), so its(column, id)index serves the order and the limit.README.md,GET /api/v1/history: thebeforerow ("a message in this history") and the error "400unknown before idwhen no message with thatidis in this history" do not matchGetHistory. History returns onlyPRIVMSG, but thebeforelookup accepts the id of any message recorded for the channel or session (aJOIN,MODE,NOTICEorINVITE) and answers 200. Acceptable: the lookup also requirescommand = 'PRIVMSG', or the README says theidof a message sent to this channel or to your session.Judgement calls:
INVITE,JOIN,PART,TOPIC,MODEandKICK, which history never returns, taken as within the issue's scope.Model: opus-5-5
bd5caeaba9to48d6308c22Rework for #161 (comment), amended into the single commit and rebased onto
next:ERR_NOSUCHNICK, as for an unknown nick, on both transports; a test ininternal/servicedeletes the target as the DM is stored. A channel message whose channel is deleted the same way now getsERR_NOSUCHCHANNEL, with its own test.to_channel_idfor a channel and byto_session_idfor your own nick, so each column's(column, id)index serves the order and the limit.beforelookup also requirescommand = 'PRIVMSG';TestHistoryUnknownBeforenow also passes the id of aJOIN.Model: opus-5-5
FAIL (needs rework)
internal/handlers/api_test.go, forGetHistoryandqueryHistoryininternal/db/queries.go: no test pages a nick's DM history or passesbeforefor it. DM and channel history used to share one statement, whichTestHistoryPagingcovered; now theto_session_idstatement inqueryHistoryand theto_session_idmatch in thebeforelookup run only for DMs, and no test would notice if either ignoredbeforeor accepted theidof a DM sent to another session. Acceptable: a handler test that pages a nick's DM history across at least two pages, with a nick change between the DMs (covering theREADME.mdsentence on earlier nicks), and that gets400unknown before idfor theidof a DM sent to a different session.Judgement calls:
Model:line, taken as within the length limit.nextsuch a channel message was stored and answeredsent.Model: opus-5-5
48d6308c22toed00694211Rework for #161 (comment), amended into the single commit and rebased onto
next:TestHistoryDMPagingpages a nick's DM history in three pages, with a nick change after the first two DMs and a DM to another session in between, and gets400for theidof that other session's DM asbefore. The paging loop ofTestHistoryPagingmoved into a helper both tests use. The test exposed no bug in the code.PR body: the deleted-recipient disclosure now covers both DMs and channel messages and no longer reads as a fix to code on
next; trimmed.Model: opus-5-5
PASS
Model: opus-5-5