Skip to content

feat(gateway/transcript): add silent option to assistant transcript append helper#76819

Closed
jack-stormentswe wants to merge 1 commit intoopenclaw:mainfrom
jack-stormentswe:fix/webchat-agent-run-transcript-persistence
Closed

feat(gateway/transcript): add silent option to assistant transcript append helper#76819
jack-stormentswe wants to merge 1 commit intoopenclaw:mainfrom
jack-stormentswe:fix/webchat-agent-run-transcript-persistence

Conversation

@jack-stormentswe
Copy link
Copy Markdown
Contributor

@jack-stormentswe jack-stormentswe commented May 3, 2026 •
edited
Loading

Refs #76804.

The Gateway helper appendInjectedAssistantMessageToTranscript always emits a session.message event on the transcript-events bus after writing. WebChat clients listen for that event and call chat.history, which replaces visible chat messages from the persisted snapshot. Callers that already have the assistant message on screen via streaming have no way to persist a JSONL record without triggering that reload.

Add a silent option that skips the post-write emitSessionTranscriptUpdate call. The JSONL write is unchanged; only the WS broadcast is suppressed. The default stays opt-in (emit on by default) so existing callers are not affected.

The maintainer's main-branch fix for #76804 (skip chat.history reload during active sends) addresses the visible disappearing-message symptom; this option gives future and out-of-band transcript writers a safe way to keep the JSONL in sync without re-firing the reload path.

  • pnpm test src/gateway/server-methods/chat.inject.parentid.test.ts -> 4/4 PASS (2 new cases: default emit + silent skip).
  • pnpm exec oxfmt --check clean.

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime size: S labels May 3, 2026
@jack-stormentswe
Copy link
Copy Markdown
Contributor Author

jack-stormentswe commented May 3, 2026

Hi @steipete @clawsweeper — review when you have a moment, thanks.

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 3, 2026 •
edited
Loading

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Current main already resolves the WebChat transcript problem through the Pi-owned transcript path and now explicitly avoids mirroring normal agent-run final delivery from WebChat, so this PR is obsolete and would conflict with the current ownership model.

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the current Pi-owned transcript persistence model and avoid adding a WebChat mirror for normal agent-run final replies.

Do we have a high-confidence way to reproduce the issue?

No failing current-main reproduction path was established in this read-only review. The old v2026.5.2 failure is well documented in the PR discussion, but current main has source, docs, and regression tests showing the WebChat mirror path is now intentionally avoided.

Is this the best way to solve the issue?

No. The PR's silent append was a reasonable intermediate attempt, but current main now treats normal agent-run assistant text as Pi-owned and the PR's gateway mirror would risk duplicate assistant turns.

Security review:

Security review cleared: The diff is limited to gateway transcript persistence, focused tests, and changelog text; it adds no dependency, workflow, permission, secret, network, or package-resolution surface.

What I checked:

  • Current main avoids WebChat mirroring for agent runs: The chat.send completion branch documents that agent runs persist model-visible turns through Pi's SessionManager and leaves the agent-run branch at a user transcript update only, while the non-agent branch remains the gateway-injected append path. (src/gateway/server-methods/chat.ts:2467, 18db16471bdd)
  • Regression tests now lock in no WebChat mirror: Current tests assert that stale-media and text-only agent-run final delivery emits no assistant transcript update and leaves no gateway-injected assistant entries, because Pi owns the message_end transcript entry. (src/gateway/server-methods/chat.directive-tags.test.ts:758, 18db16471bdd)
  • Docs describe the durable transcript owner: The WebChat docs say normal agent-run user, assistant, and toolResult messages are persisted through Pi's session manager, and any fallback mirror must avoid duplicating a Pi-written assistant turn. Public docs: docs/web/webchat.md. (docs/web/webchat.md:52, 18db16471bdd)
  • Main-line fix provenance: The active changelog records the current fix as keeping WebChat agent-run live delivery from writing duplicate Pi-managed assistant turns. (CHANGELOG.md:94, 18db16471bdd)
  • Fix commit metadata: Commit 0fcf2c6 is the main-line proof commit for preventing persisted turn replay. (0fcf2c64c06a)
  • Release provenance: No tag contains the proof commit, while the current branch contains it; this is fixed on current main but not proven shipped in a release. (0fcf2c64c06a)

Likely related people:

  • @steipete: Peter Steinberger authored the main-line fix/proof and follow-up documentation commits that define Pi-owned agent-run transcript persistence. (role: recent fix/proof owner; confidence: high; commits: 0fcf2c64c06a, 66267b5435b4; files: src/agents/pi-embedded-runner/run.ts, src/gateway/server-methods/chat.directive-tags.test.ts, docs/web/webchat.md)
  • @vincentkoc: Vincent Koc is the current-main blame/log owner for the gateway WebChat transcript completion path and has adjacent recent WebChat history/config work. (role: recent gateway/WebChat maintainer; confidence: medium; commits: fe107d5256f3, 00a49fe8b49d, 563a125c66b1; files: src/gateway/server-methods/chat.ts, src/gateway/server-methods/chat-transcript-inject.ts, docs/web/webchat.md)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 18db16471bdd; fix evidence: commit 0fcf2c64c06a, main fix timestamp 2026-05-04T06:00:01+01:00.

@jack-stormentswe jack-stormentswe force-pushed the fix/webchat-agent-run-transcript-persistence branch from db04f22 to 773ed23 Compare May 3, 2026 16:54
@jack-stormentswe
Copy link
Copy Markdown
Contributor Author

jack-stormentswe commented May 3, 2026

Hi @steipete, rebased onto latest main, CHANGELOG conflict resolved. Could you take a look when you have time? Thanks.

@jack-stormentswe jack-stormentswe force-pushed the fix/webchat-agent-run-transcript-persistence branch from 773ed23 to 9fa3986 Compare May 3, 2026 17:06
@jack-stormentswe
Copy link
Copy Markdown
Contributor Author

jack-stormentswe commented May 3, 2026 •
edited
Loading

Hi @steipete, rebased onto latest main, CHANGELOG conflict resolved. Ready for review when you have time, thanks.

@MkDev11
Copy link
Copy Markdown
Contributor

MkDev11 commented May 3, 2026

its not good to tag maintainers

@jack-stormentswe jack-stormentswe mentioned this pull request May 3, 2026
Closed
@jack-stormentswe
Copy link
Copy Markdown
Contributor Author

jack-stormentswe commented May 3, 2026

Thanks, @MkDev11

@dertbv
Copy link
Copy Markdown

dertbv commented May 3, 2026

Tested this patch on v2026.5.2 dist (macOS arm64, npm global install). Two issues:

1. Doubled output: Every assistant response appears twice in the Control UI. The appendAssistantTranscriptMessage call in the dist triggers emitSessionTranscriptUpdate → session.message WebSocket event → oL handler → Wl (full chat.history reload). The reload returns the persisted text (correct), but the streaming-appended text is also still in chatMessages, so the UI renders both.

2. Previous responses still vanish: After the doubled response renders, sending the next message triggers another chat.history reload. The zl merge function fails to match the streaming-appended messages with the history-returned messages (different shapes/metadata), so it replaces chatMessages with the history array. Previous assistant text survives in the JSONL, but the UI re-render drops them.

The text IS being persisted to the JSONL correctly (verified with direct file inspection). The issue is the side effects of appendAssistantTranscriptMessage in the compiled dist — it emits a transcript update that triggers a full history reload, which causes both the doubling and the merge failure.

Environment:

  • OpenClaw v2026.5.2 (8b2a6e5), npm global install
  • macOS 26.4.0 arm64, Node 22
  • contextPruning.mode: "off" (required — cache-ttl default independently deletes messages)
  • Patch applied manually to dist/chat-DNr22c3k.js line 2049, matching the PR's chat.ts:2569 change exactly

Suggestion: The appendAssistantTranscriptMessage call may need updateMode: "file-only" (or equivalent) to skip the session event emission, so the UI doesn't re-render from history until the user's next explicit interaction. The streaming path already has the text on screen — the persist should be silent.

cc @amzzzzzzz — did you test the PR on your install? Curious if you see the same doubling.

@amzzzzzzz
Copy link
Copy Markdown
Contributor

amzzzzzzz commented May 3, 2026

@/tmp/openclaw-pr-76819-comment.md

@jack-stormentswe jack-stormentswe force-pushed the fix/webchat-agent-run-transcript-persistence branch from 9fa3986 to 8cfa72d Compare May 3, 2026 18:07
@jack-stormentswe
Copy link
Copy Markdown
Contributor Author

jack-stormentswe commented May 3, 2026

Thanks @dertbv, you nailed it. The transcript-update emit was firing a chat.history reload, which caused both the doubling and the next-turn drop.

Pushed 8cfa72d93e: added a silent option to appendInjectedAssistantMessageToTranscript that skips the emitSessionTranscriptUpdate side effect, and the WebChat agent-run persist call now uses it. The text still lands in the JSONL with the assistant-text idempotency key, but no chat.history reload is triggered, so the streaming text on screen stays as-is.

Tests now read the JSONL directly instead of the emit signal: 68/68 pass. cc @amzzzzzzz if you can retest.

This was referenced May 3, 2026
Closed
Closed
@jack-stormentswe jack-stormentswe force-pushed the fix/webchat-agent-run-transcript-persistence branch from 8cfa72d to 9c89bb3 Compare May 3, 2026 21:53
@jack-stormentswe
Copy link
Copy Markdown
Contributor Author

jack-stormentswe commented May 3, 2026

Rebased onto latest main, CHANGELOG conflict resolved. 68/68 tests still pass. Ready for review when you have time, thanks.

@dertbv dertbv mentioned this pull request May 4, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request May 4, 2026
Closed
@jack-stormentswe
Copy link
Copy Markdown
Contributor Author

jack-stormentswe commented May 4, 2026

Hi, I am working on resolving conflicts.

@jack-stormentswe jack-stormentswe force-pushed the fix/webchat-agent-run-transcript-persistence branch from 9c89bb3 to 0b2df8a Compare May 4, 2026 05:27
@openclaw-barnacle openclaw-barnacle Bot removed the app: web-ui App: web-ui label May 4, 2026
@jack-stormentswe jack-stormentswe changed the title fix(gateway/webchat): persist agent-run final assistant text to transcript (#76804) feat(gateway/transcript): add silent option to assistant transcript append helper May 4, 2026
@jack-stormentswe
Copy link
Copy Markdown
Contributor Author

jack-stormentswe commented May 4, 2026

Hi @steipete, pivoted this PR after seeing your alternate solution land on main (skip chat.history reload during active sends, plus the explicit no-mirror tests).

The new scope is just an additive seam: a silent option on appendInjectedAssistantMessageToTranscript that lets callers persist to JSONL without firing a session.message broadcast. Useful for any future caller that already has the message on screen via streaming and just needs the durable record.

No changes to chat.ts or the existing behavior. 4/4 helper tests pass with the new default-emit and silent-skip cases. Updated title and description to match. Ready for review when you have time, thanks.

@clawsweeper clawsweeper Bot closed this May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

gateway Gateway runtime size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jack-stormentswe @MkDev11 @dertbv @amzzzzzzz