Maintainer/Codex found one narrow edge in the keyboard intent stamp: because the
listener runs capture-phase and accepts any focused descendant of #messages,
Space/Spacebar on an in-transcript control (tool-card toggles, copy buttons,
role buttons, links/tabs) stamped _lastMessageKeyScrollIntentMs before the
control handler could preventDefault/stopPropagation. That made a button
activation look like a scroll intent and could mask a legitimate post-render
artifact.
Fix by excluding Space/Spacebar when the event target or active element is an
interactive transcript control: button, a[href], select, summary, role=button,
role=tab, role=menuitem, or contenteditable. Keep PageUp/PageDown/arrows/Home/End
stamping unchanged, and keep the existing INPUT/TEXTAREA/contenteditable composer
exclusion.
Add a behavioral node-harness test: Space on a focused transcript button leaves
the key-scroll stamp at -Infinity (serialized null), while PageUp on the pane
still stamps 1234. test_4856 + test_4295 green (20 passed), node --check clean.
Keyboard scrolling of the message pane (PageUp/PageDown, Arrow keys, Space,
Home/End) fires a native scroll event with no wheel/touch/scrollbar/non-message
intent. Inside the 1400ms post-render artifact window the suppression branch
then returned before movedUp could unpin, so a keyboard scroll-up was swallowed
and live-follow snapped the reader back to the bottom.
- Add _lastMessageKeyScrollIntentMs + _recentMessageKeyScrollIntent(), stamped
by a capture-phase keydown listener on the scroll keys, gated to when the
message pane is the scroll target (focused/contains focus/hovered) and not an
editable field (composer/input/contenteditable).
- Gate the post-render suppression on !_recentMessageKeyScrollIntent() (typeof
guard keeps the #4295 node harness inert).
- Clear the stamp in both _resetScrollDirectionTracker() and
_resetStreamScrollFollow() (same stale-state hygiene as the wheel stamp).
Tests: keyboard-scroll-inside-window-still-unpins behavioral regression (harness
extended with injected _recentMessageKeyScrollIntent), plus a source lock for the
helper/keydown-stamp/suppression-gate/both-resets. test_4856 + test_4295 green
(19 passed), node --check clean.
Maintainer dual-gate found two stale-state leaks in the new
_lastMessageWheelIntentMs and one adjacent pre-existing gap; all three fixed:
MUST-FIX 1 — _resetScrollDirectionTracker() (session switch) did not clear
_lastMessageWheelIntentMs, so a gentle wheel in chat A left
_recentMessageWheelIntent() true into chat B's first post-render window,
under-suppressing the artifact and falsely unpinning. Now reset to -Infinity.
MUST-FIX 2 — _resetStreamScrollFollow() (fresh stream) had the same leak: a
gentle upward wheel within 1200ms of a new stream could silently disable live
follow. Now reset to -Infinity.
SHOULD-FIX 3 — the suppression branch ignored _scrollbarDragActive, so a manual
scrollbar-drag upward scroll inside the 1400ms window was swallowed. Gate the
branch on (typeof _scrollbarDragActive==='undefined' || !_scrollbarDragActive);
typeof guard keeps the #4295 node harness inert.
Tests: scrollbar-drag-inside-window-still-unpins behavioral regression (harness
extended with injected _scrollbarDragActive), plus source locks for both resets
and the scrollbar-drag gate. test_4856 + test_4295 green (17 passed),
node --check clean.
Maintainer dual-gate (Codex) found the suppression could swallow a genuine
low-delta trackpad wheel scroll-up for ~1.4s after a render:
_recordNonMessageScrollIntent() only recorded message-pane wheel intent at
deltaY<-30, so a gentle deltaY:-5 left both intent helpers false and the
post-render branch returned before movedUp set _messageUserUnpinned.
- Track recent low-delta upward message-pane wheel intent separately
(_lastMessageWheelIntentMs / _recentMessageWheelIntent), recorded for any
upward wheel (deltaY<0). The decisive deltaY<-30 sticky-unpin is unchanged.
- Require !_recentMessageWheelIntent() before suppressing the artifact, so a
real gentle scroll-up inside the window still unpins.
- Add behavioral node-harness regressions: gentle wheel inside the window
unpins; no-intent artifact inside the window stays suppressed; outside the
window unpins. Plus a source lock that low-delta intent is tracked.
The new movedUp suppression branch called _recentMessageRenderArtifactWindow()
and the intent helpers in an unguarded position. The #4295 unit harness injects
the scroll-listener body via new Function() without those helpers, so the branch
threw ReferenceError on a movedUp sample and crashed the node subprocess.
Reorder the && chain with typeof-function guards first; production evaluates the
real helpers, the harness short-circuits before any call. Behavior unchanged.
Cron run output is never authored Markdown -- it is logs/JSON/plain text.
Using renderMd() with any heuristic or synthetic fence is fragile:
- Content starting with #, |, > becomes headings/tables/blockquotes
- Embedded triple-backtick in content breaks the synthetic fence
Fix: use DOM-created pre/code with textContent throughout:
- No heuristic, no synthetic fence -- every byte renders verbatim
- Both render paths (initial load + View full output) use the same approach
When viewing cron job run logs (prompt/response), the raw content
was passed through renderMd() which broke formatting for JSON and
plain text (single newlines collapsed, special chars interpreted).
Now detects if content looks like markdown and wraps raw content
in triple backticks so it renders as a proper code block with
preserved whitespace and formatting.
On the default local in-process backend, an approval card whose stream
ended while still pending (cancel/fork/provider-error/completion) left the
agent queue entry dropped and the _pending mirror reconciled away. Clicking
Approve/Deny then sent an approval_id that matched nothing, so the handler
returned a bare {ok:false}; since #4771 the frontend surfaces that as
'Approval response not accepted.' with a stuck card (reported by
@santastabber on v0.51.666 and b3nw).
Distinguish a genuinely stale card (no pending approval for the session ->
benign {ok:true, stale_cleared:true} so the UI clears the orphan) from a
stale-id click made while a DIFFERENT approval is live (still ok:false so it
can never resolve the wrong command -- #527 guard preserved). Frontend
clears the orphan card on stale_cleared even if the displayed id drifted.
Co-authored-by: b3nw <b3nw@users.noreply.github.com>
Codex+Opus gate (both reproduced): the new mixed-content ordered render path
resolved tool snippets only from the S.messages join (_collectToolResultSnippetsByTid)
and never consulted persistedSnippetByTid (S.session.tool_calls) — so on a
cold/paginated load where the tool_result join misses (#4927 scenario) the inline
card rendered an EMPTY body, AND its tid was added to transparentOrderedToolIds,
suppressing the post-loop derived card that WOULD have recovered via #4927. Build
a transparentPersistedSnippetByTid map from S.session.tool_calls and thread it as a
5th arg to _transparentOrderedToolCall, falling back in both the live and non-live
branches. Also gate the transparent-only scans (S.toolCalls + _collectToolResultSnippetsByTid)
behind isTransparentStream() so non-transparent renders skip the O(messages×parts)
work (Opus perf finding). Redaction (#4926/#4928) was already safe (shared buildToolCard).
+ cold-load persisted-fallback regression test.
A settled turn that recorded no live duration fell back to
(Date.now()/1000) - session.pending_started_at. But pending_started_at is the
start of an IN-FLIGHT turn, so for a settled turn it is either stale (a session
that sat idle) or about a different pending turn — rendering a bogus
'Processed 15h 32m' on fresh conversations (#4930). Gate the fallback on a turn
actually being in flight (active_stream_id || pending_user_message); otherwise
return undefined (no duration shown) rather than a fabricated one. The recorded
live duration (_turnDuration / base.turn_duration) still takes priority, so this
only removes bogus durations, never a correct one.
Codex gate (the key one): the persistedSnippetByTid fallback read S.session.tool_calls,
but loadSession() keeps the messages=0 session object (tool_calls []), and the
real messages=1 summary arrives only as the sessionToolCalls argument to
_syncToolCallsForLoadedMessages() — never copied onto S.session. So the fallback
source was empty on exactly the cold-load path it repairs. Persist
sessionToolCalls onto S.session.tool_calls in _syncToolCallsForLoadedMessages
(covering _ensureMessagesLoaded, older-message paging, and full-history loaders)
so renderMessages builds a populated persistedSnippetByTid. + guard test.
Codex gate: the third derived-push site (Anthropic tool_use content-array branch,
ui.js) still read resultsByTid[tid]||'' without the persisted fallback, so
Anthropic-format sessions could still cold-reload with an empty tool body. Add
the persistedSnippetByTid fallback there too; now all derived result-snippet
reads (OpenAI top-level, Anthropic tool_use, partial) chain to the durable
persisted snippet. Added a guard test asserting EVERY derived resultSnippet read
includes the fallback.
In the transparent stream, terminal output and patch diffs intermittently
vanished after a cold/paginated reload. A settled tool row renders its body from
`snippet`, but the raw-assistant-envelope 'derived' rebuild only joined the
result by tool id via resultsByTid (role:tool messages); when that join missed
(id mismatch, recovery-rebuilt turn) the snippet was empty, and #4622's live
enrichment only helps when S.toolCalls still matches (empty on cold load).
Build a persistedSnippetByTid lookup from the durable session.tool_calls compact
summary (which carries a bounded result snippet per tid) and use it as a
fallback in all three derived-push sites, so the persisted record is the
reliable source and the live join becomes a fast-path, not the only path that
shows the body. Part of #4925.
Codex gate: raising content args from 120->4000 chars newly surfaces secrets on
non-first lines / past char 120 in render paths that print tc.args directly and
bypass the #4926 label redaction. Apply _redactToolTargetLabel to arg values at
every such site: the transparent Full-tab args render (_transparentToolDetailHtml),
the non-transparent tool-card args block (buildToolCard visibleArgs), the
clipboard copy path (_copyEventToClipboard), and the _toolArgsSnapshot itself.
+ node regression proving a secret past char 120 in a long command is masked in
the Full tab while the non-secret command structure is retained.
Tool-call args were truncated to 120 chars at three sites (_truncate_tool_args,
the live _tool_args_snapshot in api/streaming.py, and _toolArgsSnapshot in
static/ui.js). For incidental args that's fine, but content/diff-bearing keys
(command/cmd/script/code/patch/diff/old_string/new_string/content/path/
file_path) feed the rendered tool card AND the diff reconstruction in
recovery-rebuilt sessions, so the cap silently corrupted long commands/paths and
produced broken/empty diffs. Exempt those keys with a much larger cap
(_TOOL_ARG_CONTENT_CAP = _TOOL_RESULT_SNIPPET_MAX = 4000) at every site, keyed by
key name; incidental args keep the 120 cap; storage stays bounded. Part of #4925.
Codex re-gate: the env/AUTH '=' redaction regex matched 'token=' INSIDE a URL
(?token=...&mode=debug&signature=...) before the dedicated URL-query redactor,
and its \S+ value swallowed the rest of the URL incl later params + closing
quote. Anchor the env-assignment match to a shell-token boundary (^|[\s;|(]) so
it can't match keys preceded by ? or & — URL query params are left to the
dedicated query-param redactor, which masks the secret-looking ones while
preserving the command tail. + regression tests (URL-tail-preserved,
benign-PATH-not-over-redacted).
Codex gate: exposing the full multi-line command in the expanded shell card
newly surfaces secrets on non-first lines (the collapsed header only ever showed
line 1). The old redactor masked only password-shaped fields. Broaden it to mask
common secret forms across the WHOLE command: env/colon assignments for
*TOKEN/*API_KEY/*SECRET/*ACCESS_KEY/*PRIVATE_KEY/*CREDENTIAL/*CLIENT_SECRET/
*SESSION_KEY and *AUTH*=, --token/--api-key/--secret style flags,
Authorization: Bearer/Bot/Token headers, and secret-looking URL query params.
Test now drives the REAL redactor (not an identity stub) and asserts non-first-line
secrets are masked while the collapsed header stays first-line.
In the transparent activity stream, a shell tool card's collapsed header
correctly shows only the first line of the command, but the EXPANDED detail
lead (`$ <command>`) reused that same first-line-only value via
_toolTargetLabel, so a multi-line script still rendered as line 1 when expanded.
The full command is available frontend-side; the truncation was purely in the
label derivation.
Adds _toolFullCommandLabel(tc) (full multi-line, redacted, no first-line
collapse) and uses it in _toolDetailLeadText for the shell kind, while the
collapsed header keeps the first-line _toolTargetLabel. Part of #4925.
Inline code pass converts `code` to <code>...</code> BEFORE the table
pass runs, so the _protectPipes backtick regex never matched. Changed
to protect pipes inside <code>...</code> tag pairs instead.
The _protectPipes function protected | inside brackets but not inside
backtick inline-code spans, so || in `code || here` inside a table
cell was split into extra columns. Added a second replace for matched
backtick pairs in the same iterative loop.
The smd parser treated | characters as table column separators even
when inside backtick inline code (token 11). Added e.token!==m (code
span) guards in both table | handlers so pipes inside code are
output as literal text instead of creating table cells.
Codex gate caught that the enrich only restored when the settled snippet was
fully EMPTY — but the backend persists a 4000-char capped preview
(_TOOL_RESULT_SNIPPET_MAX), so long terminal/tool output settles to a truncated
4000-char prefix, not empty (the actual #4622 symptom). Now also restore when
the settled snippet is a bounded preview of the live one (live longer + settled
is a prefix + settled length >= the 4000 cap), without clobbering a genuinely
short non-preview value. Adds capped-preview restore + short-prefix-not-clobbered
tests (proven non-vacuous).
Opus + Codex both SAFE TO SHIP. Opus flagged two cosmetic redundancies (not
blockers): the snippet guard checked payload.result/payload.output which
_anchorSceneToolRowFromCall never sets, and the tool.result/tool.output writes
are never read by the consumer _anchorSceneToolCallFromRow (it reads
tool.snippet||payload.snippet||payload.result||payload.output). Removed both —
tool.snippet/payload.snippet is what actually restores the visible body.
Behavior-identical; 18 settled/reconnect tests still green.
In the Transparent Stream / Worklog view, terminal tool cards lost their stdout
and patch/edit cards lost their diff AFTER a turn settled (live cards were fine).
Root cause (proven via a node harness driving the real settled chain
_anchorSceneRowsByMessageIndex -> _anchorSceneToolRowFromCall ->
_anchorSceneToolCallFromRow): the settled rebuild builds each tool row from the
persisted messages[].tool_calls (state.db/sidecar), which can carry only a short
preview — or, on a cold/paginated load, nothing — for the result body. The full
output lives on the live S.toolCalls entry at settle time, but the dedup loop in
_anchorSceneRowsByMessageIndex SKIPPED the matching live entry (continue) instead
of restoring the missing body onto the surviving settled row. So buildToolCard
got tc.snippet='' -> empty output region, and the patch/edit diff (detected from
the snippet) never rendered. This also blanks the transparent 'Output' tab and
the 'Full' (detailed-input) tab.
Fix: new _enrichSettledToolRowBodyFromLive() restores snippet/result/output,
command, and args onto a settled row from the matched live call, ONLY when the
settled value is empty (never clobbers a genuinely persisted body). Wired into
the dedup loop: on a tid match that would have been skipped, enrich the existing
pooled row instead of dropping the live data.
The prior #4625 guard only exercised the LIVE buildToolCard path, which is why it
stayed green while the settled path regressed. New
tests/test_issue4622_settled_toolcard_body.py drives the full SETTLED chain and
asserts: terminal stdout restored + full output in data-full + transparent Output
tab non-empty; patch diff body restored + renders as a diff + Full-tab args kept;
genuine persisted body not clobbered; args restored when dropped; no-live-match
leaves the row untouched; no duplicate row after enrich. Verified to FAIL on clean
master (assertions) and pass with the fix. Also registered the new helper in the
and now references the new symbol).
Reusable proof harness saved to ~/WebUI/qa/harnesses/.
Closes#4622.
Gate (Codex) caught two SILENT regressions in the original fix: explicitly
hiding the header in the 'else'/'empty' branches also hid populated read views.
- _setMemoryHeaderButtons('read') now shows the header for ALL read views
(incl. read-only Project Context / External Notes), gating only the Edit btn.
- _renderProfileConceptHelp() now uses a new 'help' header mode (shows header,
hides all action buttons) instead of 'empty' (which hid the help title).
Adds tests/test_issue4891_detail_header_visibility.py (proven non-vacuous).
Co-authored-by: luandnh <luandnh@users.noreply.github.com>
