[bong-water-water-bong]

os/v3/puppy: the chat page retries the NPU's own refusal (503 device_busy), because Lemonade does not

Follow-up to #925, on top of main after it merged. engine#924 (app/unified.cpp) bounds the wait for the one
device: a request that cannot have it within --queue-ms (default 5000) is refused with a status instead of
parked silently — and Lemonade's proxy does not retry that on the request path, so the layer that can is the
page. Before #924 a queued request looked exactly like a stuck one (measured on the board: a 48-token probe waited
10.04 s behind a generation and then answered in 51 ms, while Lemonade's proxy gave up at ~6 s — the c17 board's
"0 chunks in 5.9 s", #911).

What the page now does

os/v3/puppy/embed/opt/1bit/webapp/index.html retries a refusal after the delay the engine advertises, up to
RETRIES = 3, and says so on its status line (NPU busy, retrying (1/3) in 1 s). After the last attempt it stops
with the engine's own words (NPU busy: still refused after 4 attempts (the NPU is busy with another request) - press Send again in a moment) instead of hanging or reporting zero chunks.

It recognises both shapes #924 can produce, because this page streams:

shape	when	how it arrives
503 + Retry-After: 1 + {"error":{"message":…,"type":"device_busy"}}	a request refused before the engine starts answering	HTTP status, read through 1bit-webapp-proxy, which already forwards the upstream status and every non-hop-by-hop header
one data: {"error":{"message":"the NPU is busy with another request"}} frame, then data: [DONE]	a streaming refusal (app/unified.cpp closes every refusal properly, because a stream without [DONE] is what Lemonade reports as a hung backend)	in-stream; it carries no type, so the rule is: an error frame is retried when it says "busy" or when the stream had not produced a single chunk yet

A stream that fails after it has produced content is reported, not repeated — a mid-answer failure must not be
silently requested again.

Retry-After is clamped to 10 s so a future engine cannot park a demo, and defaults to 1 s when the header is
absent (the streaming shape has none).

Proven in a real browser, against both documented shapes

~/wt/os3i/c18-page-probe.py grew a refusal mode; Pale Moon under Xvfb, screenshots in ~/wt/os3i/c18-busy-*:

BUSY_MODE=stream BUSY_N=2   requests: {refused stream 1}, {refused stream 2}, {model …, enable_thinking false}
                            screen: "NPU busy, retrying (1/3) in 1 s"  ->  the answer
BUSY_MODE=503    BUSY_N=1   requests: {refused 503 1}, {model …}
                            screen: "NPU busy, retrying (1/3) in 1 s"  ->  the answer
BUSY_MODE=stream BUSY_N=9   4 attempts, then "NPU busy: still refused after 4 attempts … press Send again"

The probe also caught a real bug before any build saw it: the predicate was named busy, the in-flight flag is
also busy, var busy = false overwrote the function and the page answered TypeError: busy is not a function.
Renamed to isBusy; the screenshots above are from after the fix.

Gate

U3 webapp page now also requires device_busy and the NPU busy, retrying text in the page the running proxy
serves, and prints busy=N retry-text=N with the other four counts (webapp-thinking-picker-busy).

Also here: the CodeQL finding on #925's diff

?api= made a fetch URL depend on a query parameter (client-side request forgery, low). It was a cycle-15 debug
affordance nothing in the image used — the launcher opens http://localhost:8080/ with no query string — and a way
to make the appliance's own browser POST at another host. It is gone: the paths are literal same-origin
/api/v1/models and /api/v1/chat/completions, so no tainted value reaches a request URL. ?model=, ?think= and
the one-shot ?key= stay (none of them becomes a URL), and the key has not travelled in a URL since cycle 17.
The finding's thread on #925 was answered with this plan and resolved.

Not claimed

• The shipped payload does not emit this yet. The c18 stick carries lemonade-onebit-rc-2026-11-03-c21, whose
  engine predates npu serve: a request that cannot have the device is told so (engine#911) #924, so on that image the retry is inert — there is nothing to refuse. The two shapes above are
  npu serve: a request that cannot have the device is told so (engine#911) #924's documented behaviour, reproduced by the probe, not a payload measurement.
• No row can make the engine produce a refusal on demand (that needs a second concurrent generation against the
  NPU, i.e. a device ticket and a second client), so the end-to-end proof arrives with the first payload that
  carries npu serve: a request that cannot have the device is told so (engine#911) #924.
• Nothing else changes: no gate lowered, no engine change, and the reasoning/enable_thinking half was already
  os/v3/puppy: cycle 18 — the chat page answers with thinking off and a model picker, the first headless screen says how to connect, and the desktop hands the screen back to the menu #925's.
• The U3 webapp page row's new two counts are asserted against the served page; they go green with the next
  image built from main. No image was rebuilt for this PR (a page and a row), so nothing here re-measures the c18b
  artifact.

[bong-water-water-bong]

#926 ready for review at 1a3cff9 — follow-up to #925 (merged as 5baf38a): the chat page retries engine#924's refusal (both shapes: 503 + Retry-After, and the streaming error frame before [DONE]) up to 3 times with 'NPU busy, retrying (n/3) in 1 s', and ?api= is gone so no request URL carries a user value (the CodeQL finding on #925's diff). Proven in Pale Moon against both refusal shapes and the exhaustion case; U3 webapp page now requires the two strings in the page the running proxy serves.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 8c40893)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 925 - Partially compliant Compliant requirements: The chat page answers with thinking off and a model picker The first headless screen says how to connect The desktop hands the screen back to the menu The page asks for enable_thinking: false by default The page keeps a "show thinking" switch that sends true and renders reasoning dimmed above the answer The page raises max_tokens from 512 to 1024 The page names the two shapes that used to look identical: reasoning and no answer, and the stream ended without [DONE] A model picker is built from /api/v1/models, filtered to downloaded The model picker is labelled GPU / NPU from the model's own recipe_options.onebit_backend ?model= still pins one; an id that is not on the machine is ignored The console menu's prompt carries a CONNECT block above the menu with hostname, OpenAI-compatible base, key, ssh, and Lemonade Mobile's server URL While the wire or Lemonade is still coming up, it names what it is waiting for with a spinner and redraws every 2 s instead of 15 No address yet is its own line pointing at menu 3 The menu starts X on VT3 and runs chvt 1 when the session ended, saying so in the boot's log Non-compliant requirements: None Requires further human verification: None 924 - Partially compliant Compliant requirements: A request that could not have the device was never told so The device wait is bounded with --queue-ms (default 5000) A request that cannot have the device gets 503 + Retry-After: 1 with {"type":"device_busy"} A streaming refusal still closes its stream with [DONE] --queue-ms 0 refuses at once The option is refused if negative rather than clamped Non-compliant requirements: None Requires further human verification: None 911 - Partially compliant Compliant requirements: Root-cause and fix it (engine#924 addresses this) Non-compliant requirements: Reproduce on Ubuntu with the c21 package Separate the TDR from any thinking artifact Add a long-prompt-then-follow-up row to the release check's NPU chat Requires further human verification: Reproduce on Ubuntu with the c21 package Separate the TDR from any thinking artifact Add a long-prompt-then-follow-up row to the release check's NPU chat

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential retry logic flaw The retry logic in the chat page's ask() function may not correctly handle all failure scenarios. Specifically, when a streaming refusal occurs (a data: {"error":...} frame followed by [DONE]), the code retries only if no chunks have been processed yet. This could lead to retries being skipped in cases where the stream fails after some content has been received but before the full response is complete. The retry mechanism should be reviewed to ensure it handles all possible failure modes correctly. // Retry ONLY on a stream that has produced NOTHING - no answer chunk and no reasoning chunk - // because a retry clears this turn's two output elements: doing it after any text would // silently replace what the reader already has, and reasoning-only output is text too // (engine#924's own refusal shape is a reasoning-only stream). A failure after output is // reported and the output is kept. if (!chunks && !rchunks) throw busyErr(1000, why); throw new Error(why + " (after " + (chunks + rchunks) + " chunk(s); the partial answer is kept)"); } Security concern with URL parameter handling The code removes the key parameter from the URL using history.replaceState, but it does not sanitize or validate the value before using it. If an attacker can inject malicious content into the key parameter, it could potentially be used in a reflected XSS attack or other security issues. While the key is used only internally, it's still a potential vector for security issues. var key = p.get("key") || ""; if (p.get("key")) { try { history.replaceState({}, "", location.pathname); } catch (e) {} }

[bong-water-water-bong]

VERDICT: CHANGES at 1a3cff917 — one item, and it is a sentence in the body that the code does not implement: a busy-worded error frame arriving after content is retried, which also erases the answer the reader can already see.

I tested the shipped retry logic rather than reading it: the page's own <script> extracted verbatim and driven under node with a stubbed DOM/fetch (~/wt/reviews/test926.js, 9 cases). Eight are right:

OK  503 device_busy x1, then a stream       requests=2  stat="done: 3 answer chunks …"      retry: (1/3)
OK  503 device_busy x3, then a stream       requests=4  retry: (1/3) (2/3) (3/3)
OK  503 device_busy x9 (never succeeds)     requests=4  stat="NPU busy: still refused after 4 attempts (device_busy) - press Send again in a moment"
OK  in-stream busy frame x2, then ok        requests=3
OK  401 is not retried                      requests=1  stat="failed: Error: 401 - the API key is wrong or missing …"
OK  503 with Retry-After but no type        requests=2
OK  ?api= cannot move the request           url=/api/v1/chat/completions        (search was ?api=http://evil.example)
OK  stream ends without [DONE] (content seen) requests=1  stat="… - the backend ended the stream without [DONE]"

The ninth is the item:

BAD busy frame AFTER content is reported, not retried
    requests=2  stat="done: 1 answer chunk …"

The code's rule is

if (j.error) {
  var why = …;
  if (isBusy(j) || /busy/i.test(why) || !chunks) throw busyErr(1000, why);

so the /busy/i disjunct fires regardless of chunks — an error frame whose message says "busy" is retried even when two content chunks have already been rendered. The body says the opposite one paragraph later ("A stream that fails after it has produced content is reported, not repeated — a mid-answer failure must not be silently requested again"), while the table above it says "retried when it says 'busy' or when the stream had not produced a single chunk yet". The two sentences disagree for exactly this case and the code implements the table.

It matters more than a wording nit because of what a retry does to the screen: tryOnce() starts with th.textContent = ""; an.textContent = "";, so the partial answer the reader is looking at is blanked and the turn is re-asked — my run ends with "1 answer chunk", the first attempt's two chunks gone.

Which way I would settle it: the sentence, not the table — retry only when nothing has been produced (isBusy(j) || (!chunks && /busy/i.test(why))). #924's refusal is pre-stream by construction (the engine decides before it answers), so a mid-stream "busy" is not a shape this page has to survive, and silently re-asking over visible text is the worse failure. Whichever correction you take, the body and the code have to say the same thing.

Everything else checks out:

• ?api= is gone and the CSR property is now testable: with ?api=http://evil.example in the search string the request still goes to /api/v1/chat/completions. The paths are literals, ?key=/?model=/?think= never become URLs. CodeQL is SUCCESS on this head.
• Retry-After handling: absent → 1 s, 0 → immediate, clamped at 10 s (Math.min(s,10)*1000), and parseInt of a missing header lands on the 1 s default.
• The busy/isBusy shadowing bug they mention is not present in this head: var busy is the in-flight flag and isBusy the predicate, and my 401 case shows the flag still settles.
• The gate row is honest about what it can see: vm-check.sh now requires device_busy and NPU busy, retrying in the page the proxy serves (busy=/retry-text= counts, expected token webapp-thinking-picker-busy), and both strings are in the page (device_busy ×4, NPU busy, retrying ×2) — the row greps the served bytes, not the source tree.
• Not-claimed section is right: this payload's engine predates #924, so on the c18 stick the retry is inert; the two shapes are reproduced against a probe.

CI at this head: build, Analyze×4, CodeQL SUCCESS, hw_gate SKIPPED, pr_agent CANCELLED (its own superseding run).

[bong-water-water-bong]

#926 at b43be33 — one more commit, because #921 merged the ratatui console TUI while this was open: /etc/inittab's tty1 is now the TUI when the image carries the binary and the dialog menu otherwise, and the two renderers differ in exactly the way a row-grep cares about (dialog collapses runs of spaces, ratatui does not → the TUI's screen says api http://… with two spaces). U2 menu connect now reads tty1 through tr -s ' ', measured on both renders (80x25 TUI snapshot + dialog menu at 80x25).

[github-actions]

Persistent review updated to latest commit b43be33

[Copilot]

🟡 Changes recommended

The retry logic can restart a stream after answer or thinking output has begun, silently clearing the partial response instead of preserving it and reporting the error.

1 open issue

• Busy retries can overwrite partial stream output · New

What changed in this PR

This PR adds bounded retries to the Puppy chat page for the NPU refusal responses introduced in #924 and removes the user-controlled API URL override.

Changes:

• Handles HTTP and streaming refusals with retry status and exhaustion messages.
• Keeps API requests same-origin by removing ?api= support.
• Extends served-page checks for retry support and normalizes menu spacing across renderers.

File	Description
os/​v3/​puppy/​vm-check.sh	Checks retry markers in the served page and normalizes menu whitespace.
os/​v3/​puppy/​embed/​opt/​1bit/​webapp/​index.html	Adds refusal retries and removes the API URL override.

_{Review effort: Balanced}

[github-actions]

Persistent review updated to latest commit 5dde8c0

[bong-water-water-bong]

Evidence for 5dde8c05, from the browser probe (a mock that speaks engine#924's streaming refusal shape):

• reasoning produced, then the refusal (BUSY_MODE=rterr): exactly one request — no retry — and the screen keeps the text: Thinking first, then the device went away stays, with [Error: the NPU is busy with another request (after 3 chunk(s); the partial answer is kept)] beneath it.
• no output at all, then the refusal (unchanged): two refusals then the answer (stream/2), one 503 then the answer (503/1), both showing NPU busy, retrying (1/3) in 1 s.

[bong-water-water-bong]

VERDICT: READY at 5dde8c05b — both of my items are fixed, and I re-ran the harness that found them. CLEAN, build green.

1. The retry rule now matches the body's sentence. The condition is no longer a word heuristic:

var why = (j.error && j.error.message) || "the backend reported an error";
if (!chunks && !rchunks) throw busyErr(1000, why);
throw new Error(why + " (after " + (chunks + rchunks) + " chunk(s); the partial answer is kept)");

Re-run of the same harness (~/wt/reviews/test926.js, the page's own <script> extracted verbatim and driven under node with a stubbed fetch/DOM), 10 cases — the one that failed my review now passes, and the case your new comment raises also passes:

OK   busy frame AFTER content is reported, not retried      requests=1
     stat="failed: Error: the NPU is busy with another request (after 2 chunk(s); the partial answer is kept)"
OK   busy frame after REASONING only is not retried         requests=1
     stat="failed: Error: the NPU is busy with another request (after 2 chunk(s); the partial answer is kept)"
OK   503 device_busy x1 / x3 / x9                            2 / 4 / 4 requests, "(1/3)(2/3)(3/3)", then "still refused after 4 attempts"
OK   in-stream busy frame x2, then ok                       3 requests
OK   401 is not retried                                     1 request
OK   503 with Retry-After but no type                       2 requests
OK   ?api= cannot move the request                          url=/api/v1/chat/completions with ?api=http://evil.example in the search
OK   stream ends without [DONE] (content seen)              1 request, "… - the backend ended the stream without [DONE]"
OK   near-normal stream                                     done, chunk count

So: a refusal retries only when the stream produced nothing at all — reasoning included — and after any output the partial answer is kept and the message says so. That is stricter than the table in the body was, and it is the right end of the ambiguity I flagged.

2. The CONNECT row no longer depends on how many spaces a renderer emits (b43be33b2). I re-ran the row's own word list and address regex against both real renderings — the dialog menu's collapsed one and the TUI's double-spaced one:

render 1 (dialog, runs collapsed):   missing=[none] address-regex=ok
render 2 (ratatui, two spaces kept): missing=[none] address-regex=ok

tr -s ' ' is exactly the fix; with #921 merged, the TUI is what tty1 usually is now, so this closes the gap I left open on #925.

Nothing else changed that I can find: the 401 path still settles the in-flight flag, Retry-After is still clamped at 10 s with 1 s default, ?api= is still literal same-origin. build, Analyze×4, CodeQL green at this head.

[github-actions]

Persistent review updated to latest commit 8c40893