Tests that found things reading did not
The testing pass: 2140 tests to 2283, and four bugs that no amount of reading had turned up. Three came from driving the JavaScript under a Node DOM stub, which is the practice CLAUDE.md sets out and this is the reason it does. The terminal dropped every keystroke after a reconnect. `onclose` closed over the module-level socket rather than its own, and close() queues its event -- so the old socket's close arrived after a new one was assigned and nulled the live one. Output kept coming, because onmessage is bound to the object, while every send gates on the variable. It also announced "Disconnected" about a shell that had just reconnected. Two scripts were loaded twice on /messages, once by base.html and again by the page. Each is an IIFE with its own state, so four keyboard shortcuts toggled their panel twice and therefore did nothing, /help opened two dialogs, and an @ mention attached its file twice. A sweep refuses any template re-loading what base.html has. The microphone had no guard while the permission prompt was up, so each click opened another stream and only the last was ever stopped. And a skill shared with you took its name out of your own library: create checked uniqueness against what is *visible* rather than what is owned, against a (owner_id, name) constraint, and told you to edit a row you cannot edit. --ink-faint failed the contrast minimum in both themes -- 3.85 and 3.19 against 4.5 -- so the smallest text on every screen was the hardest to read. Measured in a headless browser rather than judged by eye. And the suite runs on 3.11 and 3.12 now as well as 3.14. It had only ever run on 3.14 while the image ships 3.12 and the packaging claimed 3.11: the interpreter most people would run was the one nothing had tested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,3 @@
|
||||
"""LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints."""
|
||||
|
||||
__version__ = "0.9.12"
|
||||
__version__ = "0.9.13"
|
||||
|
||||
@@ -67,12 +67,39 @@ def get(db: DBSession, skill_id: str, user: User | None) -> Skill | None:
|
||||
|
||||
|
||||
def by_name(db: DBSession, name: str, user: User | None) -> Skill | None:
|
||||
"""Look one up the way the model refers to it."""
|
||||
"""Look one up the way the model refers to it.
|
||||
|
||||
Scoped to what this person can **see**, which is theirs plus anything
|
||||
shared with them -- correct for `skill_get` and `skill_edit`, where a
|
||||
skill somebody shared is exactly what the model is reaching for.
|
||||
|
||||
It is the wrong question for "is this name taken?"; see `owned_by_name`.
|
||||
"""
|
||||
if user is None:
|
||||
return None
|
||||
return db.scalar(visible(db, user).where(Skill.name == slugify(name)))
|
||||
|
||||
|
||||
def owned_by_name(db: DBSession, name: str, owner: User) -> Skill | None:
|
||||
"""One of *this person's own* skills by name.
|
||||
|
||||
The uniqueness check used `by_name`, which is scoped to what is visible --
|
||||
so a skill somebody shared with you took that name out of your library.
|
||||
Sharing a curated skill with a team is the intended use of `library.share`,
|
||||
and doing it silently reserved the name for everyone it reached: creating
|
||||
your own was refused with "a skill called 'weekly-report' already exists.
|
||||
Edit it instead", naming a row you cannot edit, because sharing grants
|
||||
reading only. The model's `skill_create` got the same dead end.
|
||||
|
||||
The table's constraint is `(owner_id, name)`, so the question the check
|
||||
should have been asking was always this one. `documents.create_base` next
|
||||
door asks it correctly.
|
||||
"""
|
||||
return db.scalar(
|
||||
select(Skill).where(Skill.owner_id == owner.id, Skill.name == slugify(name))
|
||||
)
|
||||
|
||||
|
||||
def enabled_for(
|
||||
db: DBSession, user: User | None, *, exclude: Iterable[str] = ()
|
||||
) -> list[Skill]:
|
||||
@@ -155,7 +182,7 @@ def create(
|
||||
"A skill name must be two or more letters, numbers or hyphens, "
|
||||
"such as 'weekly-report'."
|
||||
)
|
||||
if by_name(db, slug, owner) is not None:
|
||||
if owned_by_name(db, slug, owner) is not None:
|
||||
raise SkillError(f"A skill called {slug!r} already exists. Edit it instead.")
|
||||
if not description.strip():
|
||||
raise SkillError(
|
||||
|
||||
@@ -155,7 +155,7 @@
|
||||
|
||||
--ink: #E4E8EC;
|
||||
--ink-muted: #A2ADB8;
|
||||
--ink-faint: #6E7883;
|
||||
--ink-faint: #7A848F;
|
||||
--ink-inverse: #0B0E11;
|
||||
|
||||
/* Mithril: the cool primary, used for focus and interactive accents. */
|
||||
@@ -263,7 +263,7 @@
|
||||
|
||||
--ink: #2C2419;
|
||||
--ink-muted: #6A5C48;
|
||||
--ink-faint: #94856D;
|
||||
--ink-faint: #756A55;
|
||||
--ink-inverse: #FDFBF5;
|
||||
|
||||
/* Hobbit-door blue-green: the cool primary. */
|
||||
|
||||
@@ -104,6 +104,17 @@
|
||||
return;
|
||||
}
|
||||
|
||||
/* Out of `idle` **before** awaiting permission, or the button stays
|
||||
clickable for as long as the browser's prompt is up -- and the dispatcher
|
||||
below starts a recording on every click in that window. Each one opened
|
||||
its own stream and its own MediaRecorder, only the last of which was kept
|
||||
in `recorder`, so stopping released one and left the rest live: the
|
||||
browser's recording indicator stayed on until the tab was closed.
|
||||
|
||||
`working` already exists and is already styled, so this is the state the
|
||||
button was missing rather than a new one. */
|
||||
setMicState(button, "working");
|
||||
|
||||
navigator.mediaDevices.getUserMedia({ audio: true }).then(function (stream) {
|
||||
chunks = [];
|
||||
recorder = new MediaRecorder(stream);
|
||||
@@ -124,6 +135,9 @@
|
||||
recorder.start();
|
||||
setMicState(button, "recording");
|
||||
}).catch(function () {
|
||||
// Back to idle, or a refused permission leaves a button nothing can press
|
||||
// again -- including the reader who has just gone and allowed it.
|
||||
setMicState(button, "idle");
|
||||
notify("The microphone could not be opened. Permission may be blocked.", "error");
|
||||
});
|
||||
}
|
||||
|
||||
@@ -125,10 +125,27 @@
|
||||
"?cols=" + (term.cols || 80) + "&rows=" + (term.rows || 24);
|
||||
|
||||
say("Connecting…");
|
||||
socket = new WebSocket(url);
|
||||
socket.binaryType = "arraybuffer";
|
||||
/* Held in a local as well as in `socket`, because every handler below has
|
||||
to know *which* connection it belongs to.
|
||||
|
||||
socket.onmessage = function (event) {
|
||||
`close()` queues its event rather than firing it, and both reconnect
|
||||
paths -- the `behind` frame and `repointTerminal` -- close the old
|
||||
socket and immediately call this function. So the old socket's `close`
|
||||
arrives *after* a new one has been assigned, and a handler that touched
|
||||
the module-level `socket` was reaching past its own connection into the
|
||||
live one. It set it to null: output kept arriving, because `onmessage`
|
||||
is bound to the object, while `send`, `term.onData` and the paste
|
||||
handler all gate on `socket && readyState === OPEN` -- so every
|
||||
keystroke was dropped and no resize was ever sent again, on a panel that
|
||||
looked perfectly healthy. And `closedOnPurpose` had already been cleared
|
||||
for the new connection, so it said "Disconnected. Close and reopen to
|
||||
reconnect." about a shell that had just reconnected. */
|
||||
var opened = new WebSocket(url);
|
||||
socket = opened;
|
||||
opened.binaryType = "arraybuffer";
|
||||
|
||||
opened.onmessage = function (event) {
|
||||
if (socket !== opened) return;
|
||||
if (typeof event.data === "string") return control(event.data);
|
||||
/* Written straight through as bytes. xterm's decoder is stateful across
|
||||
calls, so a multi-byte character split across two frames still lands
|
||||
@@ -136,12 +153,14 @@
|
||||
term.write(new Uint8Array(event.data));
|
||||
};
|
||||
|
||||
socket.onclose = function () {
|
||||
opened.onclose = function () {
|
||||
if (socket !== opened) return;
|
||||
socket = null;
|
||||
if (!closedOnPurpose) say("Disconnected. Close and reopen to reconnect.");
|
||||
};
|
||||
|
||||
socket.onerror = function () {
|
||||
opened.onerror = function () {
|
||||
if (socket !== opened) return;
|
||||
/* A failed handshake gives the page nothing: no status, no reason. So
|
||||
this is a guess, and it names the likeliest cause rather than
|
||||
pretending to know. */
|
||||
|
||||
@@ -105,8 +105,21 @@
|
||||
</div>
|
||||
{% endblock %}
|
||||
|
||||
{#
|
||||
`steps.js` only. `composer.js` and `commands.js` were listed here too, and
|
||||
`base.html` already loads both on every page -- so this screen ran each of
|
||||
them **twice**.
|
||||
|
||||
Each is an IIFE with its own state, and `stopPropagation()` does not stop a
|
||||
second listener already bound to the same node. So: two composer menus stacked
|
||||
on each other; one Enter on a highlighted `/` item running the command twice
|
||||
(`/help` opening two dialogs, `/image` posting the message twice); an `@`
|
||||
mention attaching its file twice; and `Alt+B`, `Alt+E`, `Alt+T` and `Alt+I`
|
||||
toggling their panel twice, which is to say doing nothing at all.
|
||||
|
||||
None of it looks like a script loaded twice. Found by driving the file under a
|
||||
DOM stub, which is the rule `CLAUDE.md` sets out and the reason it does.
|
||||
#}
|
||||
{% block scripts %}
|
||||
<script src="{{ url_for('static', path='js/composer.js') }}" defer></script>
|
||||
<script src="{{ url_for('static', path='js/commands.js') }}" defer></script>
|
||||
<script src="{{ url_for('static', path='js/steps.js') }}" defer></script>
|
||||
{% endblock %}
|
||||
|
||||
Reference in New Issue
Block a user