Skip to content

Add streaming support and retry logic for model providers - #9

Closed
nafeeur wants to merge 4 commits into
mainfrom
claude/browser-maskshift-6tqruo
Closed

nafeeur wants to merge 4 commits into
mainfrom
claude/browser-maskshift-6tqruo

Conversation

@nafeeur

@nafeeur nafeeur commented Sep 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

This PR adds streaming support for model output and implements robust retry logic for provider requests. Model responses now arrive token-by-token instead of all at once, with support for Server-Sent Events (OpenAI, Anthropic, Gemini) and newline-delimited JSON (Ollama) formats. Provider requests now automatically retry on transient failures with exponential backoff and Retry-After header support.

Key Changes

Streaming Support

  • Added openStream() function to establish streaming connections with idle timeout watchdog (resets on each chunk)
  • Implemented format-specific stream readers: readOpenAiStream(), readResponsesStream(), readAnthropicStream(), readOllamaStream(), readGeminiStream()
  • Added SSE and NDJSON decoders (sseData(), ndjsonData()) that handle real chunk boundaries
  • Unary response parsers (parseOpenAiBody(), parseAnthropicBody(), etc.) shared between streaming and non-streaming paths
  • Streaming gracefully falls back to unary parsing when endpoints ignore stream: true and return plain JSON
  • Added streaming config option (default true) with per-provider override support
  • TUI now displays tokens in real-time as they arrive, replacing the spinner with live text preview

Retry Logic

  • Enhanced fetchJson() with configurable retry attempts, exponential backoff with full jitter, and Retry-After header parsing
  • Defined RETRY_STATUS set (408, 409, 425, 429, 500, 502, 503, 504) to retry only transient failures; 4xx client errors (400, 401, 403, 404, 422) never retry
  • Implemented backoffMs() with full jitter over the top half of the window to break up thundering herds
  • Added sleep() function with abort signal support for backoff waits
  • Streaming requests only retry before first token; body is handed over untouched once headers are good
  • Added onRetry callback to emit model.request.retrying events with attempt count, wait time, and failure reason
  • Default retry settings: 3 attempts, 500ms base, 30s max

Testing & Validation

  • Added tests/streaming.test.mjs with comprehensive streaming tests for all provider types
  • Added tests/providers.test.mjs with retry behavior validation (transient vs permanent failures, Retry-After parsing, connection drops)
  • Added scripts/assert-no-dependencies.mjs to mechanically verify zero runtime dependencies
  • Added GitHub Actions CI workflow (ci.yml) running on Node 22 and 24

Documentation & Configuration

  • Updated CONFIGURATION.md with streaming section and retry settings
  • Updated maskshift.config.example.json with streaming and providerRetry defaults
  • Updated CHANGELOG.md with streaming and retry features
  • Regenerated capability manifest and tool documentation

Implementation Details

  • Streaming timeout is an idle watchdog reset on every chunk, not a total budget—long generations are normal, silent sockets are not
  • Tool call fragments arrive out of order and are assembled at the end by index
  • Full jitter formula: ceiling * (0.5 + Math.random() / 2) ensures spread without collapsing to near-zero
  • Timers are not unref()d during backoff—a run waiting out a backoff is real work and the process must not exit
  • Streaming detection uses content-type header to distinguish SSE/NDJSON from plain JSON responses
  • Signal abort is checked immediately after transport to surface cancellation without retry

`npm test` failed on any machine. The port_inspect scenario asserted the tool
exits 0 when inspecting an unused port, but ss, lsof and netstat all exit
non-zero when a filter matches nothing — that is "no sockets", not failure.

The throw also aborted the scenario before rsync_transfer ran, so the coverage
gate reported two tools as having no assertions when it had simply never
reached them. Scenarios now record why they aborted and the gate reports that
cause instead of blaming the tools downstream of it.

Behind the assertion sat a real bug: with neither ss nor lsof installed,
port_inspect fell through to a bare `netstat -anp` without checking that
netstat exists, returning a shell "not found" with exit 127. It now resolves
the inspector up front, errors clearly when none is installed, and reports
which one ran and whether anything matched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016boXW9QQKrmaAo3tn2FEiW
Streaming. Every provider type now streams a turn token by token instead of
returning it whole: Server-Sent Events for openai-responses, openai-compatible,
anthropic and gemini, newline-delimited JSON for ollama. Both framings are a
few lines over the fetch body and a TextDecoder, so this costs no dependency.
`maskshift run` writes tokens straight to stdout, and the interface renders the
answer being written where it used to show a spinner, repainting on the ticker
it already runs while busy. Tool-call arguments split across frames are
reassembled before parsing.

Streaming is skipped where it would mislead. The text tool protocol rewrites
content once the turn completes, so streaming it would show call syntax that
then vanishes. An endpoint that accepts `stream: true` and replies with a
single JSON body anyway — proxies and older local servers do — is detected from
its content type and parsed normally, rather than reading zero frames and
reporting an empty turn. Each provider's unary parser is now shared by both
paths so a quirk is only described once.

The stream timeout is an idle watchdog reset on every chunk rather than a total
budget: a long generation is normal, a silent socket is not.

Retries. A single 429 or dropped connection used to end a run outright —
fetchJson recorded the status code and no caller ever read it. Requests now
retry 408/409/425/429/500/502/503/504 and network errors with exponential
backoff and jitter, honouring Retry-After. Codes meaning the request itself is
wrong (400/401/403/404/422) are never retried, since the tool-protocol
downgrade depends on a 400 surfacing at once. Cancelling a run breaks out of
the backoff instead of waiting it out, a stream is only retried before its
first token so nothing is shown twice, and model-listing probes stay
single-shot so a dead endpoint is reported promptly.

Configure with `streaming` and `providerRetry`, or per provider.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016boXW9QQKrmaAo3tn2FEiW
…host

There was no .github at all. CI now runs check, test and smoke on Node 22 and
24, plus a job that regenerates docs/ and fails if the committed copy differs.

That docs job exists because `npm run docs` was publishing the generating
machine's home directory. skillsDirs includes ~/.claude/skills and
~/.codex/skills by default, so the generator documented whatever the person
running it happened to have installed: docs/SKILLS.md listed 44 skills where
the repository ships 36, and eight rows carried paths like
../../../root/.claude/skills/synced/<uuid>/docx. Generation is now scoped to
the bundled skills/ directory and refuses to emit any path outside the
repository. The generatedAt timestamp is gone from CAPABILITY-MANIFEST.json so
output is reproducible — without that the drift check could never pass. The
README's skill count and badge are corrected to 36.

Added `npm run deps` (scripts/assert-no-dependencies.mjs), which asserts the
project's central claim mechanically rather than by convention: no runtime
dependency fields in package.json, and no bare import specifier anywhere in
src/, bin/, scripts/ or tests/. Wired into `npm run verify` and CI.

Also fixed two things the version work surfaced. VERSION was hardcoded as
1.0.0 in src/core/utils.mjs while package.json said 1.0.1, so every banner,
--version, doctor report, generated document and LSP clientInfo announced the
wrong version; it is read from the manifest now, and the test that asserted
the literal reads the manifest too. And repository indexing accepted a `force`
option that #runIndex never declared — it was dropped at every call site, and
since every pass is a full rescan there was never anything to force — so it is
removed, including from the repo_index schema and `workspace index --force`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016boXW9QQKrmaAo3tn2FEiW
All three were environment assumptions that only a real runner could falsify.
None were in the streaming or retry paths — those 20 tests passed on the runner
first time.

Manifest reproducibility. Removing generatedAt was not enough: each skill entry
carried updatedAt, the SKILL.md mtime, which is checkout time on a fresh clone.
That is exactly 36 lines for 36 skills, which is what the drift gate reported.
It is dropped from the generated manifest for the same reason generatedAt was —
the file is committed and diffed — while staying on the live runtime object.
Verified by regenerating either side of `touch`ing every skill.

port_inspect. The matched flag I added last commit was wrong in the same way
the assertion it replaced was wrong: ss and netstat print their column headers
whether or not the filter matched, so "stdout is non-empty" is not "found
something", and matched came back true on a runner where ss exists. It now
counts rows past each tool's header, and reports that count. ss is not
installed in the environment I developed in, which is why only CI could catch
this.

Chromium sandbox. --no-sandbox was applied only when running as root. Chromium's
sandbox needs unprivileged user namespaces, and Ubuntu 23.10+ withholds those
through AppArmor — so a default GitHub runner is non-root *and* unable to
sandbox, and Chromium aborts with "No usable sandbox!" rather than degrading.
The uid is now one of several signals rather than the whole rule: the AppArmor
restriction and the two kernel switches are read directly. Since dropping the
sandbox genuinely reduces isolation, it is logged rather than done silently.
The predicate takes its platform, uid and file reader as arguments so every
branch is unit-tested, no host exhibiting all of them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016boXW9QQKrmaAo3tn2FEiW
@nafeeur

nafeeur commented Sep 12, 2026

Copy link
Copy Markdown
Owner Author

Closing this unmerged branch and its CI workflow as part of repo cleanup.

@nafeeur nafeeur closed this Sep 12, 2026
@nafeeur
nafeeur deleted the claude/browser-maskshift-6tqruo branch September 12, 2026 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants