The message window has colour tags on, so tview read square-bracket
text in nicks, channel names, reasons, topics and message text as
tags. It dropped that text or changed the colour of the rest of the
line.
A new escape in internal/cli/ui.go writes each "[" as "[[:]", and
app.go applies it to every part of a line that comes from the user or
the server. tview.Escape is not used: it misses bracket pairs holding
symbols such as "|", and tags with a URL.
Lines for other server commands now show the command, such as
"[MODE]", which tview had read as a tag.
Model: opus-5-5
Over the HTTP API, a PRIVMSG or NOTICE body was checked only by
decoding it into a list of strings. JSON null decodes into an empty
string there, and meta was not checked at all, so a null line, or a
meta that is not an object, reached every recipient.
handlePrivmsg now answers 400 invalid request body, before anything
else, when body is present and is not an array of strings, or meta is
present and is not an object. The check returns the lines it accepted,
and the empty-body check counts those, so body is decoded once. A
missing or empty body still gets ERR_NOTEXTTOSEND. README.md says so.
Model: opus-5-5
/query set the status bar to [connected] whatever the connection
state. Before /connect, it made the status bar say [connected] while
the client had no session.
/query now reads the connection state with the nick, as /window does,
and shows [connected] only while the client is connected,
[disconnected] otherwise. TestQueryConnectionState types /query bob,
connected and not, and checks the status bar. It names the two status
texts as constants, because the linter asks for one once a third copy
appears.
Model: opus-5-5
Each file in schema/ now describes what the HTTP API on next sends and
accepts. A numeric reply has its name in command and its number in code.
body is always an array of strings. MODE, KICK and JOIN carry their
values in body, and TOPIC needs a non-empty body. PING takes no token,
and its PONG is the HTTP response. A channel name is limited to 63
characters, and the leading # is optional from a client.
PUBKEY.json is deleted, since the server has no such command. The
section of schema/README.md that listed the differences is removed,
and the sentences it made false are corrected.
Model: opus-5-5
A line that arrives in a window that is not shown is counted as
unread, but refreshStatusBar did nothing, so the window joined the
status bar's unread list only at the next SetStatus. UI now keeps
the nick, target and connection state last passed to SetStatus, and
refreshStatusBar redraws the status bar with them.
Model: opus-5-5
A second /connect started a new poll loop without stopping the first.
The first loop then polled with the new client too, so each message
could be shown twice. It also read stopPoll without a.mu while
cmdConnect wrote it.
cmdConnect now closes the earlier loop's channel in the same locked
step that replaces the client and the cursor, and hands the new
channel to the new loop. The loop checks its channel under a.mu
before each poll and again before it moves the cursor, so a stopped
loop never polls with the new client or moves the new cursor.
Model: opus-5-5
After PASS on the IRC listener, the confirming NOTICE told the user to
reconnect with PASS and their nick. The IRC listener cannot add a
connection to an existing session, so that step does not work. The
notice now says the password is set and that another client can log in
to the session over the HTTP API with POST /api/v1/login, as the README
describes. The test now checks the whole notice text.
Model: opus-5-5
handleQuit now records the reason on the connection, and cleanup
passes it to BroadcastQuit, the same service call the HTTP API's
QUIT uses. A QUIT without a reason still gives Client quit. A
connection that ends without a QUIT still gives Connection closed.
The relay test that expected Connection closed for a bare QUIT now
expects Client quit.
Model: opus-5-5
The channel-modes parameter of RPL_MYINFO left out modes that each
transport's MODE accepts. The IRC listener now sends Himnostv, adding
H, i, o and v. The HTTP API now sends Hbiklmnostv, adding H, b and v.
Each transport keeps its own list, because they accept different
modes. The tests compare the whole parameter, so a missing or extra
mode fails them. The README and the schema notes show the new HTTP API
list.
Model: opus-5-5
Both transports sent an empty string as the user modes in RPL_MYINFO.
On the IRC listener that left an empty parameter in the middle of the
line, so a client that splits on spaces read the channel modes as the
user modes. The reply now lists o and w, the user modes the server
supports. Both transports read them from one constant in
internal/service, next to the code that decides which user modes MODE
accepts. The README and the schema notes now show what the HTTP API
sends.
Model: opus-5-5
/window set the status bar to [connected] whatever the connection
state, and Alt and a digit runs /window. Before /connect, both made
the status bar say [connected] while the client had no session.
/window now reads the connection state with the nick and shows
[connected] only while the client is connected, [disconnected]
otherwise. TestWindowConnectionState types /window 0 and presses
Alt and 0, connected and not, and checks the status bar.
Model: opus-5-5
The status bar had colour tags turned on, so tview read "[connected]"
and "[disconnected]" as tags and dropped them. It dropped all or part
of a nick, a target or a window name holding square brackets the same
way.
The status bar now has colour tags turned off. It wrote none of its
own, so every bracket shows as it is. TestStartUp checks that the
status bar shows [disconnected] at start-up. TestStatusBar checks
[connected], and checks nicks and a /query target that hold brackets.
Model: opus-5-5
Run added the start-up status bar and welcome lines through
SetStatus and AddStatus before it started the tview event loop. Both
wait for that loop, so neoirc-cli stopped before it showed its
screen. They now run on a goroutine, as typed lines do.
Run now passes a new UI to run, which holds the start-up path and
returns its error. TestStartUp calls run with a simulation screen. It
fails unless, within five seconds, the status window holds two lines:
first the one with "Welcome to neoirc-cli", then the /connect hint.
Model: opus-5-5
The Enter handler of the input line and the Alt and digit key handler
ran the typed line on the tview event loop. Every UI method that
changes the screen waits for that loop, so the client stopped
responding. Each line now runs on its own goroutine, which first waits
for the line before it, so lines keep the order they were typed in.
/window now runs off the event loop. It reads the buffer name through
the new BufferName, which runs on the loop, and switches only when
that buffer exists.
One new test drives a running UI on a simulation screen with Alt and
1, then a typed line and Enter. Another holds the first line and
checks that the second waits for it.
Model: opus-5-5
In neoirc-cli, Alt and a digit switched the window but kept the send
target, so typed text went to the window the user had left. The key
now runs /window with that digit through the input handler.
/window 0 now clears the send target. The status window has no target,
but the command used to keep the previous one.
Model: opus-5-5
config.New read the number and true/false settings with viper's
GetInt, GetFloat64 and GetBool. These return zero or false for a value
that does not parse, so a typo silently changed how the server ran.
config.New now parses these settings with strconv and checks the
duration settings with time.ParseDuration. Each number and duration
must also be finite and above zero. PORT and NEOIRC_HASHCASH_BITS may
be zero: port 0 lets the system pick a port, and 0 bits turns proof of
work off. A bad value makes config.New return an error naming the
setting and its value, and startup stops. Unset settings keep their
defaults.
Model: opus-5-5
Rewrite the description of the JSON Schema files from scratch, without
tables. Describe the fields of a message as the HTTP API sends them:
the reply name in `command` and its number in `code`. Drop the
federation section, which describes nothing the server does. List each
place where a schema file disagrees with the HTTP API, so that a reader
is not misled. The files themselves are unchanged.
Part of #190.
Model: fable-5-1 (writing); opus-5-5 (rework)
The internal/server and internal/ircserver tests picked a port by
opening a listener, reading its port and closing it. They then started
a server on that number. Another process could take the port in
between, and a test failed with "address already in use".
The IRC server tests now start the server on 127.0.0.1:0 and read the
port back from its listener. The HTTP server tests start the app with
port 0. The HTTP server now keeps the listener it binds, so the stop
test sends its long poll to the port the app was given.
Model: opus-5-5
SQLite deletes a shared in-memory database when its last connection
closes. A query cancelled while it runs makes the pool close its
connection, so a test could lose all its tables and rows at random.
The new package internal/dbtest has NewURL. It returns the URL of a
new in-memory database with a random name and holds one connection to
it open until the test ends. The db, handlers, ircserver, server and
service tests take their database URL from it. The test-support lint
rule keeps the package out of the server binary.
A new test cancels a query on such a database, checks with a TEMP
table that the connection was replaced, and checks that a row is
still there.
Model: opus-5-5
SQLite keeps the foreign keys setting per connection, and the server
turned it on once with a PRAGMA. A query cancelled while it runs makes
the pool replace its only connection, and the new one ran without
foreign keys until a restart.
connect now adds `_pragma=foreign_keys(1)` to the database URL. The
driver runs it on every connection it opens. The server refuses to
start when DBURL sets foreign_keys itself, because the driver sorts
the pragmas and that one could win.
One test cancels a query and checks, with a TEMP table, that the
connection was replaced and that foreign keys are on. Another checks
the startup error for several spellings.
Model: opus-5-5
When an IRC listener connection ends, serve cancels the context its
relay uses. If the cancel came while the relay was reading the queue,
the SQLite driver interrupted the query and reported the connection
broken, and the pool closed its only connection. In the tests that
deletes the in-memory database, so in TestIntegrationKill alice never
got bob's QUIT. In production the new connection runs without
PRAGMA foreign_keys = ON.
The relay now reads the queue without that cancel. It still stops at
its next check of the context.
Model: opus-5-5
The handlers tests ran one at a time because config.New writes the
global viper instance. Each test spends most of its time in SQLite
under the race detector, so run in a row they took longer than the
20-second target.
Every test and subtest now calls t.Parallel(), as the ircserver and
db tests already do. A mutex in newTestServerWith makes tests start
their servers one at a time, since config.New writes viper and the
server reads it as it starts. No test is removed or otherwise
changed.
Model: opus-5-5
Co-authored-by: clawbot <sneak+clawbot@sneak.cloud>
openDM took the direct-message tab's position from the tab list of the
render its caller came from. /msg calls it only after the PRIVMSG
request returns. By then the server's copy of the message, or a join or
part from another client of the session, may have changed the tabs. The
active tab then became another tab or none, and the next typed line went
to the wrong place or was dropped.
openDM now finds the tab by name inside its setTabs update and makes it
active there, as joinChannel does.
Model: opus-5-5
The web client stored each tab's messages and unread count under the
tab's name, and the Server tab was named Server. A user with the nick
Server got a direct-message tab with the same name, so the two tabs
showed one message list.
The Server tab is now stored under "*server*". No nick or channel name
can contain *, so that user's tab keeps its own messages. The tab bar
still labels the tab Server.
Model: opus-5-5
removeChannelTab made the Server tab active whenever it closed a
channel's tab. When a part or kick came from another client, the user
lost the channel they were reading.
It now finds the tab the user was reading by its name among the
remaining tabs and keeps it active. Only when that was the closed tab
does the Server tab show.
When the kicked nick is the user's own, the kick line also goes into
the Server tab, which counts it as unread. That copy carries the
event's id with "-server" added, since addMessage drops an id it has
already seen.
Model: opus-5-5
When the server refuses a JOIN, POST /api/v1/messages answers 200
with status "error" and sends the error numeric to the queue. Both
clients opened the channel anyway.
JoinChannel in internal/cli/api now returns an error unless the
status is "joined". cmdJoin then keeps its buffer and target and
prints its usual "Join failed" line. The web client opens no tab for
a refused join, whether it came from /join or from the saved
channels a fresh session joins again.
Model: opus-5-5
After a join request returned, joinChannel made tab number
tabsRef.current.length active. When the user's own JOIN event had
already added the tab, or the tab was already open, that index was one
past the last tab. The view then fell back to the Server tab and typed
lines were dropped.
joinChannel now finds the channel's tab by name inside its own tab-list
update and makes that tab active there. Preact runs that update at once
on the newest tab list, so the position is right even when the JOIN
event, or the close of another tab, landed first.
Model: opus-5-5
A PART event from the user's own nick now closes the channel's tab,
whether this web client or another client of the session sent it. It
adds no message or unread count.
A KICK event now goes into the channel's tab as "nick was kicked from
channel by kicker (reason)", with the kicked nick taken from params.
Before, it fell through to the Server tab, which showed only the
reason. When the kicked nick is the user's own, the tab closes.
The tab removal that partChannel did moves into removeChannelTab,
which partChannel and both events share.
Model: opus-5-5
neoirc-cli wrote its own line when it sent a message, joined or parted,
and ignored the server's event for its own nick, except a JOIN, so a
join showed twice and what the session did from another client never
showed. The HTTP API queues these events to every client of the
session, so the CLI now writes no line of its own when it sends and
shows the event as it shows anyone else's. A direct message the user
sent goes in the recipient's buffer. The user's own part does not mark
the channel unread, as the user has just left it.
Model: opus-5-5
An IRC listener connection kept its own copy of its nick, so after the session's nick was changed from another client it went on sending under the old nick, which another user could then take. Now, whenever the relay passes on a NICK line and after the connection's own NICK is stored, the connection takes the nick its session holds in the database, read by session ID under the connection's lock. Taking the nick from the database rather than from the line means a connection can never follow another user's rename and send as them. Handlers read the nick under the same lock.
Untested: an HTTP API rename landing while the connection's own NICK is being stored; that order cannot be produced reliably.
Model: opus-5-5
The IRC listener's relay dropped every line sent under the connection's own nick, so a user on the web client and an IRC client at once never saw on IRC what they did from the web client. The relay now drops nothing by nick. Instead the fan-out skips only the sending client's own queue: the IRC listener passes its own client, whose handler already wrote the echo, and the HTTP API skips none. Channel messages and PARTs now reach the sender's other clients, so an HTTP client gets its own PART event; neoirc-cli ignores it, as it already shows its own part. README.md names the commands covered.
Judgement call: a PRIVMSG or NOTICE sent on an IRC connection is not echoed back to it, as in IRC.
Model: opus-5-5
The test slept 500 ms, had one user send PING, and then asked for sessions last seen before the current time minus 500 ms. On a loaded host the PONG could arrive more than 500 ms later, so the active user counted as idle and the build went red. The code was right: activity is recorded before the reply and the stored time keeps nanoseconds. The test now reads the clock once after both users register and before the PING, and uses that as the cutoff, so the result follows from the order of events.
Judgement call: the test relies on the wall clock not stepping backwards between the cutoff and the PING.
Model: opus-5-5
An invite for an IRC listener user arrived as a server NOTICE that did
not name the inviter, so no IRC client treated it as an invite. The
relay now sends it as an INVITE line from the inviter, with the channel
taken from the message body, and sends nothing when the body names no
channel. The IRC listener now queues the INVITE with the channel as its
body, as the HTTP API already did, so an invite from either transport
has the same shape. The README's IRC listener section says how an
invite arrives.
Model: opus-5-5
The web client saved its channel tabs to localStorage whenever the
tabs changed, including on page load, when only the Server tab
exists. That wrote an empty list over the saved one before login
read it, so a new session never rejoined anything. The save now
waits until the user is logged in, and login reads the saved list
before it marks the user logged in.
A MODE message from the server fell through to the Server tab as its
bare body. It now shows in the channel's tab as "alice sets mode
+v bob on #chan".
Model: opus-5-5
The service sends a TOPIC, MODE or KICK to every channel member, the
issuer included, and the IRC listener also echoes it to the issuer's
connection, so an IRC listener user got each of their own changes
twice. The relay now drops one whose sender is the connection's own
nick, as it already does for PRIVMSG, NOTICE, JOIN, PART, NICK and
QUIT. The direct echo stays: it carries the real hostmask and arrives
in order. The README MODE section no longer describes a second copy.
Model: opus-5-5
The web client was written against an older shape of the API.
MODE now carries its change in `body`, so /mode sets modes; /mode
alone queries them and the 324 reply is shown. Numeric replies are
matched on `code`, so /list, /who and /whois print their lines, and
353 names are read as hostmasks. The member list takes @ and + from
`isOperator` and `isVoiced`. A 401 on the poll reloads the page, which
shows the login screen, where it used to retry for ever. The token
code is gone: no Authorization header, nothing stored, and the page
always asks /state whether it can resume a session.
Model: opus-5-5
GET /api/v1/messages read the client output queue and only then
registered with the broker. Notify wakes only the channels registered
at that moment, so a message queued between the empty read and the
registration waited out the whole long-poll timeout. The handler now
registers first and removes its channel on every exit path; the IRC
relay loop registers before each drain in the same way.
The handlers read the queue through a function field that defaults to
Database.PollMessages, so tests can act just after a read: one checks
that a message queued there comes back at once, others that no wait
channel stays registered however a request ends.
Model: opus-5-5
bcrypt cannot hash a password longer than 72 bytes. PASS over the HTTP
API answered one with a 500, and the IRC listener confirmed it without
storing it.
Both transports now refuse a password outside 8 to 72 bytes with the
reply a short one already got: 461 over the HTTP API, a NOTICE over the
IRC listener. setPassword returns its error, and "Password set" is sent
only when the password was stored. A PASS given before registration, an
empty one included, is checked after the welcome burst and gets the
same NOTICE.
Model: opus-5-5
A session request refused with 400 or 409, or a +H channel message
refused for membership, a ban or +m, used to spend its stamp, so a
retry with the same stamp was refused as already used.
hashcash.Validator.Validate is split into Check, which spends
nothing, and Claim, which records the stamp; Release undoes a claim.
Session creation checks the stamp first, claims it just before
CreateSession and releases it if CreateSession fails.
SendChannelMessage checks the stamp after the membership, ban and +m
checks.
Model: opus-5-5
acceptLoop called newConn, which did the reverse DNS lookup, with its
three-second timeout, before the loop could accept the next
connection, so one client whose address had no answering reverse DNS
held up every other IRC connect. serve now does the lookup at its
start, on the connection's own goroutine, and the accept loop only
accepts.
The server keeps the lookup function in a field, as it does the ping
times, so that a test can give it one that blocks. The new test
checks that a second client registers while the first one's lookup
has not returned.
Model: opus-5-5
The CLI read the poll cursor from `lastId` while the server sends
`last_id`, so the cursor stayed at 0 and every poll returned the
whole queue at once. It now reads `last_id`.
/names decodes the member objects the server sends and prints each
nick with `@` for an operator or `+` for a voiced member. /list
prints each channel's name and topic, since the server sends no
member count. StateResponse.Channels decodes channel objects.
/topic with no text prints a usage line and sends nothing, because
the HTTP API has no topic query and a TOPIC without text gets 461.
The help says "Set topic".
The client tests serve the bodies the handlers send, so a wrong JSON
tag in types.go fails a test; another test covers the /names prefixes.
Model: opus-5-5
Logout deleted the client, counted the session's clients and ran
BroadcastQuit as separate statements, and BroadcastQuit read the peers,
parted, deleted empty channels and deleted the session separately too.
Two logouts or QUITs of one session at once both sent its QUIT, and a
JOIN landing between a part and the channel's deletion got into a +k
channel without the key.
db.QuitSession now does all of that in one transaction, parting each
channel as PartChannelAndDeleteIfEmpty does, and stores and queues the
QUIT in it only when it deleted the session, so the QUIT is queued
before the nick is free. db.LogoutClient deletes the client and counts
the rest in the same transaction.
Model: opus-5-5
The HTTP server ran ListenAndServe in a goroutine, so a failed bind
was only logged and neoircd kept running without its HTTP API.
The start hook now binds the listen address and returns the bind
error, so neoircd exits 1 with an error naming the address. If serving
the bound listener later ends with anything other than a normal
shutdown, the server stops the app through fx.Shutdowner with exit
code 1; the handler tests now pass it that Shutdowner.
Tests check that a taken port fails startup, that a serve failure
stops the app with exit code 1, and that a normal stop does not. The
server tests no longer run in parallel: building the app writes the
global viper instance.
Model: opus-5-5
Stopping the server returned before the IRC listener's connections had
run their cleanup, and the database closed under them, so their
sessions, clients and channel memberships stayed and their nicks stayed
taken after a restart.
The IRC listener's stop now waits up to five seconds for every
connection's cleanup, which now runs on a context that stopping does
not cancel. The HTTP server's stop hook now runs Shutdown itself and
waits for it; httpServer is set before serving starts, and the server's
own signal watcher, which raced fx's stop hooks, is gone.
Model: opus-5-5
JoinChannel made the channel, counted its members, ran the join
checks and inserted the member as separate statements, so two
requests could each decide on a count the other was about to change:
+l was exceeded, two first joiners both became operator, and a JOIN
racing the last PART failed its foreign key and answered 500.
db.CreateOrJoinChannel now does all of that, and uses up the invite,
in one transaction; the join checks moved into it, and the service
turns its refusals into the same IRC errors as before. PART and KICK
remove the member and delete an empty channel in one transaction
(db.PartChannelAndDeleteIfEmpty). The db functions this leaves with
no caller are removed.
Model: opus-5-5
setupSPA took the file name from request.URL.Path[1:], which panics
when the path is empty, as it is for an absolute-form request line
with no path ("GET http://host HTTP/1.1"). It now trims a leading "/"
instead; an empty name fails to read, so the request gets the index
page, as "/" already does.
The new test in internal/server sends such a request and expects 200
and the index page. export_test.go gives it a Server whose router has
only the web client route.
Model: opus-5-5
serve closed a connection after five minutes with nothing read and never
sent a PING, so a quiet registered client lost its session. It now sends
PING :<server> after pingInterval with nothing read, and closes the
connection when nothing is read within pongDeadline after that. Every
line read starts both waits again. readTimeout is removed.
Unregistered connections are sent the PING too, so a PONG before
registration is now ignored instead of answered with ERR_NOTREGISTERED.
Model: opus-5-5
checkChannelHashcash looked a stamp up with one statement and recorded
it with another, so concurrent requests carrying the same stamp could
all pass the lookup before any of them recorded it, and each message
was sent. RecordSpentHashcash now reports whether its INSERT OR IGNORE
added a row, and the message is refused when it did not. A single
statement is atomic in SQLite. IsHashcashSpent had no other caller and
is removed. A failed write of the stamp now refuses the message instead
of being logged and ignored.
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
The IRC relay registers a broker wait channel on each pass of its
loop, and when the 100 ms ticker fired instead of a notification it
looped without removing that channel. An idle IRC listener connection
added ten channels a second to the broker, and all but the last stayed
after the connection closed. The ticker case now removes its channel,
as the HTTP long-poll does after its timer.
Broker.ListenerCount returns how many wait channels a user holds, so
the test can check that an idle connection holds at most one, and
none after it closes.
Model: opus-5-5
Co-authored-by: clawbot <sneak+clawbot@sneak.cloud>
clawbot