Skip to content

fix(streaming): prevent dropped characters in incremental smd path#1

Merged
bsgdigital merged 1 commit intolocal-patchesfrom
fix/streaming-smd-prefix-desync
Apr 24, 2026
Merged

fix(streaming): prevent dropped characters in incremental smd path#1
bsgdigital merged 1 commit intolocal-patchesfrom
fix/streaming-smd-prefix-desync

Conversation

@bsgdigital
Copy link
Copy Markdown
Owner

Summary

  • Fix live-stream character drops caused by incremental streaming-markdown desync when display text changes are not strict append-only.
  • Add a prefix-alignment guard in _smdWrite that rebuilds the parser from full current display text when desync is detected.
  • Add regression assertions in tests/test_streaming_markdown.py for _smdWrittenText tracking and the desync guard.

Test plan

  • uvx --from pytest pytest tests/test_streaming_markdown.py tests/test_sprint48.py

Made with Cursor

Detect prefix desync between current display text and already-streamed text, then rebuild the streaming-markdown parser from full content to avoid character loss during live rendering. Add regression assertions for the new desync guard.

Made-with: Cursor
@bsgdigital bsgdigital merged commit 02ef8cc into local-patches Apr 24, 2026
@bsgdigital bsgdigital deleted the fix/streaming-smd-prefix-desync branch April 24, 2026 16:39
bsgdigital pushed a commit that referenced this pull request Apr 29, 2026
When _loadOlderMessages prepends older messages, the viewport snaps
to the bottom instead of staying where the user was.

Two bugs compounding:
1. Wrong scrollable container. Code used `$("msgInner")` for scrollHeight
   and scrollTop, but #msgInner has no overflow-y — it is a flex column.
   The actual scrollable container is #messages (`.messages{overflow-y:auto}`).
   Setting msgInner.scrollTop was silently ignored.
2. renderMessages calls scrollToBottom at the end (ui.js:2552),
   which unconditionally scrolls #messages to the bottom and sets
   _scrollPinned=true. Since bug #1 made the scroll-restore a no-op,
   the page landed at the bottom every time.

Fix:
- Changed scroll restore target from `$("msgInner")` to `$("messages")`.
- Reset _scrollPinned = false after restoring the user position,
  so scrollToBottom does not re-fire on next tick.
bsgdigital pushed a commit that referenced this pull request Apr 30, 2026
Per Opus pre-release review (SHOULD-FIX #1): the CHANGELOG claimed both
filter sites exempt 'active_stream_id OR pending_user_message', but the
index path operates on compact() output which doesn't include
pending_user_message. The behavior is correct in both paths because both
fields are set/cleared in lockstep during streaming, but the wording was
stronger than what the code does. Tightened to describe what each path
actually checks.
bsgdigital pushed a commit that referenced this pull request Apr 30, 2026
- New CONTRIBUTORS.md: full ranked credit roll for all 66 contributors
  (5+ tiers), with first/latest release versions, single-PR roll, and
  attribution methodology. Generated from git log + gh pulls API +
  CHANGELOG mention parsing.

- README.md: stack-ranked top-10 contributors table at the top of the
  Contributors section, link to CONTRIBUTORS.md for the full list.
  Updated test count (1898 → 3309). Refreshed @franksong2702 and
  @bergeouss entries to reflect their broader bodies of work (now
  the #1 and nesquena#2 external contributors).

- ARCHITECTURE.md: removed stale 'tracks upstream v0.50.36' header;
  bumped current shipped build to v0.50.245 with current architecture
  state notes (streaming-markdown vendoring, byte-range streaming,
  configurable-model-badges).

- ROADMAP.md / SPRINTS.md / TESTING.md: header/last-updated bumps to
  v0.50.245 and 3309 tests. SPRINTS.md 'Where we are now' section
  refreshed for current CLI/Claude parity (~95% Claude parity now).

Generated by aggregating CHANGELOG attribution lines, gh PR API
authors, and CHANGELOG version-section walks. Internal/bot accounts
filtered out.
bsgdigital pushed a commit that referenced this pull request May 3, 2026
…na#1458 Bug #1)

Issue nesquena#1458 reports persistent-host crashes (≥1/day) when running the WebUI
under launchd KeepAlive on macOS. Root cause: `bootstrap.py` calls
`subprocess.Popen([python, "server.py"], start_new_session=True)`, probes
/health, then exits 0. Under any process supervisor (launchd, systemd,
supervisord, runit, s6), the supervisor sees its tracked PID exit, marks
the program as "completed," and respawns it. The new bootstrap fails to
bind port 8787 (orphaned server still has it), exits non-zero, supervisor
respawns again — loop until the orphan crashes for some other reason and
the next respawn finds the port free.

This PR addresses Bug #1 of the three failure modes tracked in nesquena#1458:
the `bootstrap.py` double-fork breaking process supervisors. Bug nesquena#2
(state.db FD leak) and Bug nesquena#3 (HTTP-unhealthy wedge) remain open under
the same issue — they need diagnosis data before a fix can land.

Changes
-------

1. `bootstrap.py`:
   - New `--foreground` argparse flag with help text mentioning launchd /
     systemd / supervisord.
   - New `_detect_supervisor()` that returns the env var name for any
     supervisor it detects: `INVOCATION_ID` / `JOURNAL_STREAM` /
     `NOTIFY_SOCKET` (systemd, s6), `XPC_SERVICE_NAME` (launchd),
     `SUPERVISOR_ENABLED` (supervisord), or `HERMES_WEBUI_FOREGROUND` for
     the explicit user opt-in. Truthy values for the explicit opt-in:
     `1` / `true` / `yes` / `on` (case-insensitive).
   - `main()` branches on `args.foreground or _detect_supervisor()`:
     - **Foreground path:** chdir to `agent_dir or REPO_ROOT`, then
       `os.execv(python, [python, server_path])` to replace the bootstrap
       process image with the server. The supervisor sees the long-lived
       server as the original child. No `wait_for_health` probe — the
       supervisor's KeepAlive / Restart=on-failure handles liveness.
     - **Default path:** unchanged. Spawn server as detached child via
       `Popen + start_new_session=True`, probe /health, return 0. This
       still works for interactive `bash start.sh` invocations.
   - Resolved env vars (HOST/PORT/STATE_DIR/AGENT_DIR) are now mutated on
     `os.environ` directly instead of into a local `env` copy so they
     are inherited across `os.execv`.

2. `docs/supervisor.md` (new): runnable launchd plist, systemd .service,
   and supervisord conf examples + a diagnostic recipe (`lsof` + ppid
   chain) for catching the orphan-loop in production.

3. `.gitignore`: allowlist `docs/supervisor.md` (the directory uses an
   opt-in pattern; matches the existing `!docs/docker.md` precedent).

4. `tests/test_bootstrap_foreground.py` (new): 35 regression tests
   covering the argparse flag, `_detect_supervisor()` behavior across all
   five supervisor env vars, the explicit opt-in's truthy/falsy values,
   and `main()`'s execv-vs-Popen routing decision under each input
   combination. `os.execv` is monkeypatched in the routing tests — we
   pin the structural choice (which call is made, with which args, in
   which cwd, with which env) not the post-exec behavior.

Why this scope and no more
--------------------------

Bug nesquena#2 (state.db FD leak) lists 5 candidate paths and asks the reporter
for `lsof -p <pid> | sort | uniq -c | sort -rn | head -20` output to
disambiguate. Until that data lands, any "fix" would be speculative —
explicitly out of scope per the contributor-pickup comment on the issue.

Bug nesquena#3 (launchd-running, port-listening, HTTP-unhealthy) was added in
@stefanpieter's reply comment. Diagnosis is in flight; no concrete fix
shape yet. Also out of scope.

Running locally end-to-end verifies the behavior:

```
[bootstrap] Starting Hermes Web UI on http://127.0.0.1:8789 (foreground mode: --foreground)
$ pgrep -af 'server.py'
2997632 /home/.../python /tmp/wt-fix-1458/server.py
$ ps -o ppid -p 2997632
2997581   ← bash that ran bootstrap.py — same PID as the original bootstrap
$ ps -p 2997581 -o cmd
... bootstrap.py ...   ← but exec'd into server.py
```

The same PID that bash forked for `bootstrap.py` is now `server.py`.
A supervisor watching that PID would correctly observe the long-lived
server. No double-fork.

Verification
------------

- 3811 tests pass (`pytest tests/` — full suite, +51 from this PR plus
  master-merge-in)
- All 35 new bootstrap-foreground tests pass
- `bash scripts/run-browser-tests.sh` PASS (HTTP API checks against worktree)
- `bash scripts/webui_qa_agent.sh 8789` PASS (23/23 visual QA)
- Live verified: server starts cleanly under both `--foreground` and
  `HERMES_WEBUI_FOREGROUND=1`; PID lineage confirms no double-fork

Closes nesquena#1458 (Bug #1 only). Bugs nesquena#2 and nesquena#3 remain tracked under the
issue.
bsgdigital pushed a commit that referenced this pull request May 3, 2026
…267 follow-ups

- CHANGELOG.md: v0.50.269 entry detailing nesquena#1478 nesquena#1479 nesquena#1480
- ROADMAP.md: bump to v0.50.269, 3847 tests collected
- TESTING.md: bump header + total to 3847

nesquena#1478: nesquena APPROVED self-built bootstrap.py --foreground mode
       (closes nesquena#1458 Bug #1, +Opus follow-ups: XPC noise filter, executability guard)
nesquena#1479: surgical follow-up to nesquena#1473 — Session.compact() now includes pending_user_message
nesquena#1480: bfcache pageshow restores active session via loadSession + checkInflightOnBoot

3847 tests pass (+47 net). Opus advisor on stage diff: no blockers.
bsgdigital pushed a commit that referenced this pull request May 3, 2026
…ix + P0 polish bundle (nesquena#1466 nesquena#1494 nesquena#1469 nesquena#1484 nesquena#1486)

3 PRs in this batch (3866 → 3874 tests, +8):

- nesquena#1493 (@dso2ng) — sidebar Stop response cancels row's stream not active pane's (closes nesquena#1466, follow-up to nesquena#1480)
- nesquena#1495 (self-built; reported by @insecurejezza in nesquena#1494) — state.db connection FD leak in sidebar polling (closes nesquena#1494, addresses Bug nesquena#2 of nesquena#1458)
- nesquena#1492 (@bergeouss) — P0 bugfixes bundle: tool-card args readability + CLI rename persistence + scroll pinning + sw.js relative-path regression test (closes nesquena#1469 nesquena#1484 nesquena#1486)

This release closes Bug nesquena#2 of the umbrella issue nesquena#1458. Bug #1 was closed by v0.50.269 (nesquena#1483) + v0.50.270 (nesquena#1487). Bug nesquena#3 (HTTP-unhealthy without FD exhaustion) is the remaining work item.
bsgdigital pushed a commit that referenced this pull request May 3, 2026
…n guard)

CHANGELOG, ROADMAP, TESTING bumped (3925 → 3929 tests collected).

Opus SHOULD-FIX absorbed in-release: tests #1-3 documented the dedup
contract via direct construction but did not invoke get_models_grouped().
Test nesquena#4 (test_get_models_grouped_unconfigured_providers_get_independent_dicts)
inspects the live source for the literal copy.deepcopy(auto_detected_models)
call AND runs an end-to-end smoke of the fixed assignment loop.

A future refactor that removes the deepcopy at api/config.py:2078 will
fail this test immediately.
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.

1 participant