Merge pull request #5528 from nesquena/release/stage-5471
Release — composer draft-restore suppression, root-fixed (#5471)
This commit is contained in:
nesquena-hermes
GitHub
commit
980e98dc7e
7 changed files with 378 additions and 15 deletions
|
|
@ -14,6 +14,7 @@
|
|||
|
||||
### Fixed
|
||||
|
||||
- **The composer no longer repopulates the tail of a message you already sent.** On mobile/slow connections, a just-sent message could momentarily reappear in the composer because a stale server-side `composer_draft` restored before the empty-draft write caught up. A short per-session suppression window now blocks that stale restore right after send — but scoped to the specific sent payload, so a genuinely new draft you (or another tab) start for the same conversation still restores immediately instead of being swallowed for the window. Drafts are also cleared on the busy send paths (queue / interrupt / steer). Thanks @ruizanthony. (#5471)
|
||||
- **Windows: after an upgrade, browsers no longer keep serving stale JS/CSS when git isn't on the server's PATH.** When the WebUI server was launched on Windows from an environment without git on `PATH` (a venv, a service wrapper, a non-interactive launcher), version detection silently degraded to `unknown`, which froze the `?v=<stamp>` static-asset cache key — so browsers kept serving cached `ui.js` / `messages.js` / CSS even after the server restarted with fixed code, and the only workaround was a manual hard-refresh on every device. Version detection now resolves `git.exe` from the Git-for-Windows registry key (and common install paths) when it isn't on `PATH`, so the cache stamp updates and clients pick up frontend fixes automatically. Non-Windows behavior is unchanged. Thanks @allenliang2022. (#5522)
|
||||
- **Transparent Stream is smoother during streaming — less row churn and no thinking-block scrollbar flicker.** Building on the identity-preserving live-row reconcile, the renderer now skips redundant work while a response streams: a preserved row's markup isn't rewritten when the incoming HTML is unchanged, a row isn't reinserted when it's already in the correct position, and long thinking blocks no longer flicker an internal scrollbar as their scroll container appears. Purely a reduction of no-op DOM work — what renders is unchanged, and matching rows still preserve their DOM node, copy button, tool-call data, and expanded/collapsed state. Thanks @Stacey2911. (#5456, #5367)
|
||||
- **Models from a user-defined provider in `config.yaml` now route correctly.** If you defined a custom provider under the `providers:` block with an explicit `models:` allowlist, a bare model id from that list wasn't matched by request-time routing — it silently fell back to the default provider and often 404'd. Resolution now scans your `providers:` allowlists (exact match, same as `custom_providers:`), so those models reach the right endpoint. Copilot is deliberately excluded from this scan because `providers.copilot.models` is a per-model settings map (reasoning effort, limits), not a routing allowlist. Thanks @akay64. (#5511)
|
||||
|
|
|
|||
|
|
@ -1027,8 +1027,12 @@ function _composerTextWithPendingSelections(){
|
|||
}
|
||||
|
||||
function _clearComposerAfterQueuedSelectionSend(){
|
||||
const sid=arguments.length?arguments[0]:(S.session&&S.session.session_id);
|
||||
const composer=(typeof $==='function'&&$('msg'))||document.getElementById('msg');
|
||||
const draftText=composer?String(composer.value||''):'';
|
||||
const draftFiles=Array.isArray(S.pendingFiles)?[...S.pendingFiles]:[];
|
||||
if(composer)composer.value='';
|
||||
if(sid&&typeof _clearComposerDraft==='function') _clearComposerDraft(sid,draftText,draftFiles);
|
||||
_clearPendingSelections();
|
||||
if(typeof autoResize==='function') autoResize();
|
||||
}
|
||||
|
|
@ -1306,6 +1310,7 @@ async function send(){
|
|||
const _modelState=_chatPayloadModelState();
|
||||
queueSessionMessage(_targetSid,{text:_text,files:[...S.pendingFiles],model:_modelState.model,model_provider:_modelState.model_provider,profile:S.activeProfile||'default'});
|
||||
_clearComposerAfterQueuedSelectionSend();
|
||||
if(_targetSid&&typeof _clearComposerDraft==='function'&&_targetSid!==(S.session&&S.session.session_id)) _clearComposerDraft(_targetSid,_text,S.pendingFiles?[...S.pendingFiles]:[]);
|
||||
S.pendingFiles=[];renderTray();
|
||||
updateQueueBadge(_targetSid);
|
||||
showToast(`Queued: "${_text.slice(0,40)}${_text.length>40?'…':''}"`,2000);
|
||||
|
|
@ -1362,6 +1367,7 @@ async function send(){
|
|||
}
|
||||
const defaultMessageMode=window._defaultMessageMode||'steer';
|
||||
if(defaultMessageMode==='steer'&&S.activeStreamId&&typeof _trySteer==='function'){
|
||||
const _steerDraftFiles=Array.isArray(S.pendingFiles)?[...S.pendingFiles]:[];
|
||||
// Real steer: clear the input first so the user gets immediate
|
||||
// feedback, then ship the steer payload via /api/chat/steer.
|
||||
// _trySteer restores the draft and leaves the active stream running if
|
||||
|
|
@ -1373,12 +1379,13 @@ async function send(){
|
|||
// After a delivered steer, clear any staged files because the text-only
|
||||
// steer payload has been handled. On failure, keep files staged.
|
||||
if(_steerDelivered){S.pendingFiles=[];renderTray();}
|
||||
if(_steerDelivered&&S.session&&typeof _clearComposerDraft==='function') _clearComposerDraft(S.session.session_id,text,_steerDraftFiles);
|
||||
} else if(defaultMessageMode==='interrupt'){
|
||||
// Queue the message, then cancel so drain re-sends it.
|
||||
const _modelState=_chatPayloadModelState();
|
||||
queueSessionMessage(S.session.session_id,{text,files:[...S.pendingFiles],model:_modelState.model,model_provider:_modelState.model_provider,profile:S.activeProfile||'default'});
|
||||
updateQueueBadge(S.session.session_id);
|
||||
$('msg').value='';autoResize();
|
||||
_clearComposerAfterQueuedSelectionSend(S.session&&S.session.session_id);
|
||||
S.pendingFiles=[];renderTray();
|
||||
if(S.activeStreamId&&typeof cancelStream==='function'){
|
||||
showToast(t('busy_interrupt_confirm'),2000);
|
||||
|
|
@ -1391,7 +1398,7 @@ async function send(){
|
|||
// 'steer' mode when no stream is active or _trySteer is unavailable.
|
||||
const _modelState=_chatPayloadModelState();
|
||||
queueSessionMessage(S.session.session_id,{text,files:[...S.pendingFiles],model:_modelState.model,model_provider:_modelState.model_provider,profile:S.activeProfile||'default'});
|
||||
$('msg').value='';autoResize();
|
||||
_clearComposerAfterQueuedSelectionSend(S.session&&S.session.session_id);
|
||||
S.pendingFiles=[];renderTray();
|
||||
updateQueueBadge(S.session.session_id);
|
||||
showToast(`Queued: "${text.slice(0,40)}${text.length>40?'…':''}"`,2000);
|
||||
|
|
@ -1577,7 +1584,8 @@ async function send(){
|
|||
}
|
||||
}
|
||||
if(!msgText){setComposerStatus('Nothing to send');return;}
|
||||
|
||||
const _submittedDraftTextForClear=$('msg').value||'';
|
||||
const _submittedDraftFilesForClear=Array.isArray(_failedSendFilesSnapshot)?[..._failedSendFilesSnapshot]:[];
|
||||
$('msg').value='';autoResize();
|
||||
// Clear persisted composer draft since message was sent. Capture the promise
|
||||
// so the #5472 failed-send restore can chain its re-persist AFTER this clear
|
||||
|
|
@ -1585,7 +1593,7 @@ async function send(){
|
|||
// text:<draft>) can be reordered under HTTP/2 multiplexing and leave the
|
||||
// server draft empty after a reload. (Opus #5484 NIT.)
|
||||
let _composerDraftClearPromise=null;
|
||||
if (activeSid && typeof _clearComposerDraft === 'function') _composerDraftClearPromise=_clearComposerDraft(activeSid);
|
||||
if (activeSid && typeof _clearComposerDraft === 'function') _composerDraftClearPromise=_clearComposerDraft(activeSid,_submittedDraftTextForClear,_submittedDraftFilesForClear);
|
||||
const displayText=_slashDisplayTextOverride||text||(uploaded.length?`Uploaded: ${uploadedNames.join(', ')}`:'(file upload)');
|
||||
const userMsg={role:'user',content:displayText,attachments:uploaded.length?uploadedNames:undefined,_ts:Date.now()/1000};
|
||||
S.toolCalls=[]; // clear tool calls from previous turn
|
||||
|
|
|
|||
|
|
@ -34,6 +34,112 @@ let _draftSaveTimer = null;
|
|||
const _DRAFT_SAVE_DELAY_MS = 400;
|
||||
const NEW_CHAT_DRAFT_SESSION_KEY = 'hermes-new-chat-draft-session';
|
||||
const _composerDraftKnownPayloadSessions = new Set();
|
||||
const _composerDraftRestoreSuppressedUntilBySid = new Map();
|
||||
const _COMPOSER_DRAFT_RESTORE_SUPPRESS_MS = 30000;
|
||||
|
||||
function _composerDraftFileSignature(file) {
|
||||
if (typeof file === 'string') return { value: file };
|
||||
if (!file || typeof file !== 'object') return { value: String(file || '') };
|
||||
return {
|
||||
name: String(file.name || file.filename || ''),
|
||||
path: String(file.path || ''),
|
||||
size: Number.isFinite(Number(file.size)) ? Number(file.size) : null,
|
||||
type: String(file.type || file.mime || ''),
|
||||
};
|
||||
}
|
||||
|
||||
// A live browser `File` JSON-serializes to `{}`, so a draft persisted to the
|
||||
// server loses its name/size/type. Canonicalize files to a plain serializable
|
||||
// shape BEFORE both persisting and signing, so the suppression signature of the
|
||||
// just-sent payload matches the signature of the same payload after it has
|
||||
// round-tripped through the server draft (otherwise a text+attachment send never
|
||||
// matches its own suppression and the stale tail can repopulate — #5471).
|
||||
function _composerDraftFilesForPersist(files) {
|
||||
if (!Array.isArray(files)) return [];
|
||||
return files.filter(Boolean).map((file) => {
|
||||
if (typeof file === 'string') return file;
|
||||
if (!file || typeof file !== 'object') return String(file || '');
|
||||
const canon = {
|
||||
name: String(file.name || file.filename || ''),
|
||||
path: String(file.path || ''),
|
||||
size: Number.isFinite(Number(file.size)) ? Number(file.size) : null,
|
||||
type: String(file.type || file.mime || ''),
|
||||
};
|
||||
if (Number.isFinite(Number(file.lastModified))) canon.lastModified = Number(file.lastModified);
|
||||
return canon;
|
||||
});
|
||||
}
|
||||
|
||||
function _composerDraftPayloadSignature(text, files) {
|
||||
const normalizedText = String(text || '');
|
||||
const normalizedFiles = _composerDraftFilesForPersist(files).map(_composerDraftFileSignature);
|
||||
return JSON.stringify({ text: normalizedText, files: normalizedFiles });
|
||||
}
|
||||
|
||||
function _composerDraftPayloadSignatureForSid(sid) {
|
||||
if (typeof S === 'undefined' || !S.session || S.session.session_id !== sid) return null;
|
||||
const draft = S.session.composer_draft || null;
|
||||
if (!draft) return null;
|
||||
return _composerDraftPayloadSignature(draft.text, draft.files);
|
||||
}
|
||||
|
||||
function _suppressComposerDraftRestoreAfterSubmit(sid, text, files) {
|
||||
if (!sid) return;
|
||||
const previous = _composerDraftRestoreSuppressedUntilBySid.get(sid);
|
||||
// Collect EVERY signature a stale poll could legitimately echo back for this
|
||||
// just-sent turn, and suppress a restore matching ANY of them (#5471):
|
||||
// - the submitted-payload signature (final textarea content on send), AND
|
||||
// - the REMEMBERED SERVER DRAFT signature — what was actually persisted last.
|
||||
// The two differ in the common Enter-to-send case: `_clearComposerDraft`
|
||||
// cancels the pending debounced save, so the server's last draft is often a
|
||||
// PREFIX of the submitted text (pause ≥400ms, then send within 400ms of the
|
||||
// last keystroke) — an exact submitted-text match would miss that prefix and
|
||||
// let it restore. The remembered-server-draft signature must be read BEFORE
|
||||
// the `_rememberComposerDraftPayloadState(sid,'',[])` reset below. A genuinely
|
||||
// new cross-tab draft matches neither, so it still restores immediately.
|
||||
const signatures = [];
|
||||
const _addSig = (s) => { if (s && signatures.indexOf(s) === -1) signatures.push(s); };
|
||||
_addSig(_composerDraftPayloadSignatureForSid(sid)); // remembered server draft (read first)
|
||||
if (arguments.length >= 2) {
|
||||
_addSig(_composerDraftPayloadSignature(text, files)); // submitted payload
|
||||
} else if (previous && typeof previous === 'object' && Array.isArray(previous.signatures)) {
|
||||
previous.signatures.forEach(_addSig);
|
||||
}
|
||||
_composerDraftRestoreSuppressedUntilBySid.set(
|
||||
sid,
|
||||
{ until: Date.now() + _COMPOSER_DRAFT_RESTORE_SUPPRESS_MS, signatures },
|
||||
);
|
||||
// Local state must reflect the submitted/cleared composer immediately. The
|
||||
// POST that clears the server-side draft is async; same-session refreshes can
|
||||
// otherwise race in with the old draft and repopulate the textarea.
|
||||
_rememberComposerDraftPayloadState(sid, '', []);
|
||||
}
|
||||
|
||||
function _clearComposerDraftRestoreSuppression(sid) {
|
||||
if (!sid) return;
|
||||
_composerDraftRestoreSuppressedUntilBySid.delete(sid);
|
||||
}
|
||||
|
||||
function _isComposerDraftRestoreSuppressed(sid, text, files) {
|
||||
if (!sid) return false;
|
||||
const suppression = _composerDraftRestoreSuppressedUntilBySid.get(sid);
|
||||
if (!suppression) return false;
|
||||
const until = (suppression && typeof suppression === 'object') ? suppression.until : suppression;
|
||||
if (!until) return false;
|
||||
if (Date.now() > until) {
|
||||
_composerDraftRestoreSuppressedUntilBySid.delete(sid);
|
||||
return false;
|
||||
}
|
||||
const signatures = (suppression && typeof suppression === 'object' && Array.isArray(suppression.signatures))
|
||||
? suppression.signatures
|
||||
: null;
|
||||
// Legacy/unknown callers still fail closed for the current TTL, but all send
|
||||
// paths now pass payload signatures so a different cross-tab draft can restore.
|
||||
if (!signatures || !signatures.length) return true;
|
||||
if (signatures.indexOf(_composerDraftPayloadSignature(text, files)) !== -1) return true;
|
||||
_composerDraftRestoreSuppressedUntilBySid.delete(sid);
|
||||
return false;
|
||||
}
|
||||
|
||||
function _profileMatchesActiveProfile(profile, activeProfile){
|
||||
const eventName = (typeof profile === 'string' && profile.trim()) ? profile.trim() : 'default';
|
||||
|
|
@ -101,8 +207,9 @@ function _saveComposerDraft(sid, text, files) {
|
|||
if (!sid) return;
|
||||
clearTimeout(_draftSaveTimer);
|
||||
const normalizedText = String(text || '');
|
||||
const normalizedFiles = Array.isArray(files) ? files.filter(Boolean) : [];
|
||||
const normalizedFiles = _composerDraftFilesForPersist(files);
|
||||
if (_composerDraftHasPayload(normalizedText, normalizedFiles)) {
|
||||
_clearComposerDraftRestoreSuppression(sid);
|
||||
_composerDraftKnownPayloadSessions.add(sid);
|
||||
}
|
||||
_draftSaveTimer = setTimeout(() => {
|
||||
|
|
@ -143,7 +250,10 @@ function _saveComposerDraftNow(sid, text, files) {
|
|||
if (!sid) return Promise.resolve();
|
||||
clearTimeout(_draftSaveTimer);
|
||||
const normalizedText = String(text || '');
|
||||
const normalizedFiles = Array.isArray(files) ? files.filter(Boolean) : [];
|
||||
const normalizedFiles = _composerDraftFilesForPersist(files);
|
||||
if (_composerDraftHasPayload(normalizedText, normalizedFiles)) {
|
||||
_clearComposerDraftRestoreSuppression(sid);
|
||||
}
|
||||
// Most chat switches leave an empty composer. Avoid putting the switch path
|
||||
// behind a network POST unless there is new local draft content or an existing
|
||||
// server draft that must be cleared.
|
||||
|
|
@ -174,6 +284,11 @@ function _restoreComposerDraft(draft, targetSid, opts={}) {
|
|||
const files = (draft && Array.isArray(draft.files)) ? draft.files : [];
|
||||
const current = ta.value || '';
|
||||
const preserveActiveInput = !!(opts && opts.preserveActiveInput);
|
||||
const restoreSid = targetSid || (S.session && S.session.session_id);
|
||||
const hasServerDraftPayload = _composerDraftHasPayload(text, files);
|
||||
|
||||
if (restoreSid && hasServerDraftPayload && _isComposerDraftRestoreSuppressed(restoreSid, text, files)) return;
|
||||
if (restoreSid && !hasServerDraftPayload) _clearComposerDraftRestoreSuppression(restoreSid);
|
||||
|
||||
// Same-session force refreshes are driven by external state changes and may
|
||||
// finish seconds after the user continued typing. In that case the local
|
||||
|
|
@ -202,10 +317,12 @@ function _restoreComposerDraft(draft, targetSid, opts={}) {
|
|||
}
|
||||
|
||||
// Clear the saved draft for a session (called when message is sent).
|
||||
function _clearComposerDraft(sid) {
|
||||
function _clearComposerDraft(sid, text, files) {
|
||||
if (!sid) return;
|
||||
clearTimeout(_draftSaveTimer);
|
||||
_clearRememberedNewChatDraftSession(sid);
|
||||
if (arguments.length >= 2) _suppressComposerDraftRestoreAfterSubmit(sid, text, files);
|
||||
else _suppressComposerDraftRestoreAfterSubmit(sid);
|
||||
return api('/api/session/draft', {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ session_id: sid, text: '' }),
|
||||
|
|
|
|||
|
|
@ -310,9 +310,13 @@ class TestSendBusyBranchDispatch:
|
|||
assert send_idx >= 0, "send() not found"
|
||||
steer_idx = MESSAGES_JS.find("defaultMessageMode==='steer'", send_idx)
|
||||
assert steer_idx >= 0, "busy steer branch not found"
|
||||
branch = MESSAGES_JS[steer_idx:steer_idx + 900]
|
||||
branch_end = MESSAGES_JS.find("} else if(defaultMessageMode==='interrupt')", steer_idx)
|
||||
assert branch_end > steer_idx, "busy steer branch end not found"
|
||||
branch = MESSAGES_JS[steer_idx:branch_end]
|
||||
assert "const _steerDelivered=await _trySteer" in branch
|
||||
assert "const _steerDraftFiles=Array.isArray(S.pendingFiles)?[...S.pendingFiles]:[];" in branch
|
||||
assert "if(_steerDelivered){S.pendingFiles=[];renderTray();}" in branch
|
||||
assert "_clearComposerDraft(S.session.session_id,text,_steerDraftFiles)" in branch
|
||||
assert branch.index("const _steerDelivered=await _trySteer") < branch.index("if(_steerDelivered){S.pendingFiles=[];renderTray();}")
|
||||
|
||||
|
||||
|
|
|
|||
134
tests/test_composer_draft_after_send.py
Normal file
134
tests/test_composer_draft_after_send.py
Normal file
|
|
@ -0,0 +1,134 @@
|
|||
"""Regression coverage for stale composer_draft restoration after send."""
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
SESSIONS_JS = ROOT.joinpath("static", "sessions.js").read_text(encoding="utf-8")
|
||||
MESSAGES_JS = ROOT.joinpath("static", "messages.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _block(source: str, start_marker: str, end_marker: str) -> str:
|
||||
start = source.index(start_marker)
|
||||
end = source.index(end_marker, start)
|
||||
return source[start:end]
|
||||
|
||||
|
||||
def test_clear_composer_draft_suppresses_same_session_stale_restore():
|
||||
"""An async draft-clear POST must not allow old server draft text to repopulate #msg."""
|
||||
assert "const _composerDraftRestoreSuppressedUntilBySid = new Map();" in SESSIONS_JS
|
||||
assert "function _composerDraftPayloadSignature(text, files)" in SESSIONS_JS
|
||||
assert "function _suppressComposerDraftRestoreAfterSubmit(sid, text, files)" in SESSIONS_JS
|
||||
clear_body = _block(SESSIONS_JS, "function _clearComposerDraft(sid, text, files)", "const SESSION_VIEWED_COUNTS_KEY")
|
||||
suppress_idx = clear_body.index("_suppressComposerDraftRestoreAfterSubmit(sid, text, files);")
|
||||
post_idx = clear_body.index("api('/api/session/draft'")
|
||||
assert suppress_idx < post_idx, "restore suppression must be local and immediate before async POST"
|
||||
|
||||
|
||||
def test_non_empty_draft_save_clears_submit_restore_suppression():
|
||||
save_body = _block(SESSIONS_JS, "function _saveComposerDraft(sid, text, files)", "function _composerDraftHasPayload")
|
||||
assert "_clearComposerDraftRestoreSuppression(sid);" in save_body
|
||||
now_body = _block(SESSIONS_JS, "function _saveComposerDraftNow(sid, text, files)", "// Restore composer draft")
|
||||
assert "_clearComposerDraftRestoreSuppression(sid);" in now_body
|
||||
|
||||
|
||||
def test_restore_skips_suppressed_non_empty_server_draft_only():
|
||||
restore_body = _block(SESSIONS_JS, "function _restoreComposerDraft(draft, targetSid", "// Clear the saved draft")
|
||||
assert "const restoreSid = targetSid || (S.session && S.session.session_id);" in restore_body
|
||||
assert "const hasServerDraftPayload = _composerDraftHasPayload(text, files);" in restore_body
|
||||
assert "hasServerDraftPayload && _isComposerDraftRestoreSuppressed(restoreSid, text, files)" in restore_body
|
||||
assert "!hasServerDraftPayload) _clearComposerDraftRestoreSuppression(restoreSid);" in restore_body
|
||||
|
||||
|
||||
def test_busy_send_paths_clear_persisted_composer_draft():
|
||||
helper_body = _block(MESSAGES_JS, "function _clearComposerAfterQueuedSelectionSend", "function _flushSelectionBlocksToComposer")
|
||||
assert "function _clearComposerAfterQueuedSelectionSend()" in helper_body
|
||||
assert "const sid=arguments.length?arguments[0]:(S.session&&S.session.session_id);" in helper_body
|
||||
assert "const draftText=composer?String(composer.value||''):'';" in helper_body
|
||||
assert "const draftFiles=Array.isArray(S.pendingFiles)?[...S.pendingFiles]:[];" in helper_body
|
||||
assert "_clearComposerDraft(sid,draftText,draftFiles)" in helper_body
|
||||
|
||||
in_progress_body = _block(MESSAGES_JS, "if (_sendInProgress) {", " _sendInProgress = true;")
|
||||
assert "_clearComposerAfterQueuedSelectionSend();" in in_progress_body
|
||||
assert "_clearComposerDraft(_targetSid,_text,S.pendingFiles?[...S.pendingFiles]:[])" in in_progress_body
|
||||
|
||||
busy_body = _block(MESSAGES_JS, "if(S.busy||compressionRunning){", " if(S.session&&(S.session.read_only||S.session.is_read_only))")
|
||||
assert "_clearComposerAfterQueuedSelectionSend(S.session&&S.session.session_id);" in busy_body
|
||||
assert busy_body.count("_clearComposerAfterQueuedSelectionSend(S.session&&S.session.session_id);") >= 2
|
||||
assert "_clearComposerDraft(S.session.session_id,text,_steerDraftFiles)" in busy_body, "delivered steer must clear persisted draft with the submitted payload signature"
|
||||
|
||||
|
||||
def test_file_signature_survives_server_draft_round_trip():
|
||||
"""#5471 attachment case: the signature of a just-sent text+File payload must
|
||||
MATCH the signature of the same payload after it round-trips through the server
|
||||
draft (where a live File JSON-serializes to {}). Both the persist path and the
|
||||
signature path must canonicalize files identically, or a text+attachment send
|
||||
never matches its own suppression and the stale tail repopulates.
|
||||
"""
|
||||
import json
|
||||
import shutil
|
||||
import subprocess
|
||||
import textwrap
|
||||
|
||||
node = shutil.which("node")
|
||||
if not node: # pragma: no cover
|
||||
import pytest
|
||||
pytest.skip("node not available")
|
||||
|
||||
persist_fn = _block(
|
||||
SESSIONS_JS,
|
||||
"function _composerDraftFilesForPersist(files)",
|
||||
"function _composerDraftPayloadSignature(text, files)",
|
||||
)
|
||||
sig_fns = _block(
|
||||
SESSIONS_JS,
|
||||
"function _composerDraftFileSignature(file)",
|
||||
"function _composerDraftPayloadSignatureForSid(sid)",
|
||||
)
|
||||
|
||||
harness = textwrap.dedent(
|
||||
"""
|
||||
%(sig_fns)s
|
||||
%(persist_fn)s
|
||||
|
||||
// A real browser File exposes name/size/type via PROTOTYPE getters that
|
||||
// JSON.stringify drops (serializes to {}). Simulate that: own props empty,
|
||||
// metadata on the prototype.
|
||||
function makeFile(name, size, type, lastModified) {
|
||||
return Object.create({ name, size, type, lastModified });
|
||||
}
|
||||
const liveFile = makeFile('report.pdf', 1234, 'application/pdf', 42);
|
||||
|
||||
// THE BUG: persisting the raw File loses everything through JSON.
|
||||
const rawPersistLossy = JSON.parse(JSON.stringify([liveFile])); // -> [{}]
|
||||
// THE FIX: canonicalize BEFORE persist so metadata survives the round-trip.
|
||||
const canonPersist = JSON.parse(JSON.stringify(_composerDraftFilesForPersist([liveFile])));
|
||||
|
||||
// Signature of what the server would return in each case, vs the sent payload.
|
||||
const sentSig = _composerDraftPayloadSignature('hi', [liveFile]);
|
||||
const restoredSigLossy = _composerDraftPayloadSignature('hi', rawPersistLossy);
|
||||
const restoredSigCanon = _composerDraftPayloadSignature('hi', canonPersist);
|
||||
const otherSig = _composerDraftPayloadSignature('hi', [makeFile('notes.txt', 99, 'text/plain', 7)]);
|
||||
|
||||
console.log(JSON.stringify({
|
||||
harnessOk: JSON.stringify(liveFile) === '{}',
|
||||
lossyWouldMismatch: sentSig !== restoredSigLossy, // demonstrates the bug exists
|
||||
canonMatchesSelf: sentSig === restoredSigCanon, // the fix
|
||||
differsFromOther: sentSig !== otherSig,
|
||||
}));
|
||||
"""
|
||||
) % {"sig_fns": sig_fns, "persist_fn": persist_fn}
|
||||
|
||||
proc = subprocess.run([node, "-e", harness], capture_output=True, text=True, timeout=30)
|
||||
assert proc.returncode == 0, f"node harness failed: {proc.stderr}"
|
||||
out = json.loads(proc.stdout.strip())
|
||||
assert out["harnessOk"] is True, "harness must simulate a File that JSON-serializes to {}"
|
||||
assert out["lossyWouldMismatch"] is True, (
|
||||
"sanity: persisting the raw File (the bug) loses metadata so the restored "
|
||||
"signature would NOT match the sent one"
|
||||
)
|
||||
assert out["canonMatchesSelf"] is True, (
|
||||
"the fix: canonicalizing files before persist makes a text+attachment send's "
|
||||
"signature match the same payload after the server draft round-trip — #5471"
|
||||
)
|
||||
assert out["differsFromOther"] is True, (
|
||||
"a genuinely different draft must NOT collide with the sent signature"
|
||||
)
|
||||
|
|
@ -38,11 +38,34 @@ def _draft_restore_block() -> str:
|
|||
return SESSIONS_JS[start:end]
|
||||
|
||||
|
||||
def _run_case(*, initial_text: str, draft: dict | None, opts: dict | None, current_sid, force_reload: bool) -> dict:
|
||||
def _draft_restore_suppression_block() -> str:
|
||||
start = SESSIONS_JS.index("const _composerDraftRestoreSuppressedUntilBySid")
|
||||
end = SESSIONS_JS.index("function _profileMatchesActiveProfile", start)
|
||||
return SESSIONS_JS[start:end]
|
||||
|
||||
|
||||
def _run_case(*, initial_text: str, draft: dict | None, opts: dict | None, current_sid, force_reload: bool, suppress_restore: bool | dict = False, remembered_server_draft: dict | None = None) -> dict:
|
||||
if NODE is None:
|
||||
pytest.skip("node not on PATH")
|
||||
restore_fn = _function_block(SESSIONS_JS, "function _restoreComposerDraft(draft, targetSid, opts={}) {")
|
||||
draft_block = _draft_restore_block()
|
||||
suppression_block = _draft_restore_suppression_block()
|
||||
if isinstance(suppress_restore, dict):
|
||||
suppress_call = (
|
||||
"_suppressComposerDraftRestoreAfterSubmit("
|
||||
f"sid, {json.dumps(suppress_restore.get('text', ''))}, {json.dumps(suppress_restore.get('files', []))}"
|
||||
");"
|
||||
)
|
||||
elif suppress_restore:
|
||||
suppress_call = "_suppressComposerDraftRestoreAfterSubmit(sid);"
|
||||
else:
|
||||
suppress_call = ""
|
||||
# At SEND time the session's composer_draft holds what was last persisted to
|
||||
# the server (the prefix/old draft), which the dual-signature suppression
|
||||
# reads. That is DISTINCT from the draft a later stale/cross-tab poll echoes
|
||||
# back for restore. Seed the send-time remembered draft separately, then swap
|
||||
# in the restore draft before _restoreComposerDraft runs.
|
||||
send_time_draft = remembered_server_draft if remembered_server_draft is not None else draft
|
||||
script = f"""
|
||||
const state = {{
|
||||
value: {json.dumps(initial_text)},
|
||||
|
|
@ -58,16 +81,31 @@ function autoResize() {{
|
|||
function updateSendBtn() {{
|
||||
state.updateSendBtnCount += 1;
|
||||
}}
|
||||
const S = {{
|
||||
session: {{
|
||||
composer_draft: {json.dumps(draft)},
|
||||
}},
|
||||
}};
|
||||
let _loadingSessionId = null;
|
||||
const sid = 'boot-session';
|
||||
const S = {{
|
||||
session: {{
|
||||
session_id: sid,
|
||||
composer_draft: {json.dumps(send_time_draft)},
|
||||
}},
|
||||
}};
|
||||
const currentSid = {json.dumps(current_sid)};
|
||||
const forceReload = {json.dumps(force_reload)};
|
||||
const opts = {json.dumps(opts or {})};
|
||||
function _composerDraftHasPayload(text, files) {{
|
||||
return !!(String(text || '') || (Array.isArray(files) && files.filter(Boolean).length));
|
||||
}}
|
||||
function _rememberComposerDraftPayloadState(sid, text, files) {{
|
||||
state.rememberedDraft = {{sid, text, files}};
|
||||
if (S.session && S.session.session_id === sid) {{
|
||||
S.session.composer_draft = {{text, files}};
|
||||
}}
|
||||
}}
|
||||
{suppression_block}
|
||||
{suppress_call}
|
||||
// After send-time suppression is captured, the stale/cross-tab poll delivers a
|
||||
// (possibly different) draft that restore will evaluate.
|
||||
S.session.composer_draft = {json.dumps(draft)};
|
||||
{restore_fn}
|
||||
{draft_block}
|
||||
process.stdout.write(JSON.stringify({{
|
||||
|
|
@ -123,6 +161,65 @@ def test_boot_restore_still_populates_empty_composer_from_saved_draft():
|
|||
assert data["updateSendBtnCount"] == 1
|
||||
|
||||
|
||||
def test_same_session_submitted_clear_blocks_stale_server_draft_restore():
|
||||
"""After send clears the composer, a stale same-session refresh must not refill it.
|
||||
|
||||
Mobile browsers can deliver refresh/input timing such that the textarea is
|
||||
empty locally while /api/session still returns the previous non-empty
|
||||
composer_draft. The submitted-clear suppression should keep that old draft
|
||||
out of the input.
|
||||
"""
|
||||
data = _run_case(
|
||||
initial_text="",
|
||||
draft={"text": "old submitted suffix", "files": []},
|
||||
opts={"preserveActiveInput": True},
|
||||
current_sid="boot-session",
|
||||
force_reload=True,
|
||||
remembered_server_draft={"text": "old submitted suffix", "files": []},
|
||||
suppress_restore={"text": "old submitted suffix", "files": []},
|
||||
)
|
||||
assert data["value"] == ""
|
||||
assert data["autoResizeCount"] == 0
|
||||
assert data["updateSendBtnCount"] == 0
|
||||
|
||||
|
||||
def test_same_session_submitted_clear_blocks_stale_prefix_server_draft_restore():
|
||||
"""#5471 (Opus Finding 1): the server's last persisted draft is often a PREFIX
|
||||
of the submitted text (Enter-to-send within 400ms of a keystroke cancels the
|
||||
pending debounced save), so an exact-match on the submitted text would miss it.
|
||||
The dual signature also captures the remembered SERVER draft, so the stale
|
||||
prefix is suppressed.
|
||||
"""
|
||||
data = _run_case(
|
||||
initial_text="",
|
||||
draft={"text": "hello worl", "files": []}, # stale prefix echoed back
|
||||
opts={"preserveActiveInput": True},
|
||||
current_sid="boot-session",
|
||||
force_reload=True,
|
||||
remembered_server_draft={"text": "hello worl", "files": []}, # server had the prefix
|
||||
suppress_restore={"text": "hello world!", "files": []}, # user sent the full text
|
||||
)
|
||||
assert data["value"] == "", "stale prefix of the just-sent text must be suppressed"
|
||||
assert data["autoResizeCount"] == 0
|
||||
assert data["updateSendBtnCount"] == 0
|
||||
|
||||
|
||||
def test_same_session_submitted_clear_allows_different_cross_tab_draft_restore():
|
||||
"""A different same-session draft saved by another tab must not be hidden by send-time suppression."""
|
||||
data = _run_case(
|
||||
initial_text="",
|
||||
draft={"text": "new cross-tab draft", "files": []},
|
||||
opts={"preserveActiveInput": True},
|
||||
current_sid="boot-session",
|
||||
force_reload=True,
|
||||
remembered_server_draft={"text": "old submitted suffix", "files": []},
|
||||
suppress_restore={"text": "old submitted suffix", "files": []},
|
||||
)
|
||||
assert data["value"] == "new cross-tab draft"
|
||||
assert data["autoResizeCount"] == 1
|
||||
assert data["updateSendBtnCount"] == 1
|
||||
|
||||
|
||||
def test_cross_session_restore_keeps_existing_draft_semantics():
|
||||
"""Ordinary cross-session loads should still restore the target session draft."""
|
||||
data = _run_case(
|
||||
|
|
|
|||
|
|
@ -99,12 +99,14 @@ def test_send_still_clears_composer_on_the_happy_path():
|
|||
# assertion must actually guard the main send path's clear. (Opus #5484 NIT.)
|
||||
main_clear = (
|
||||
"if (activeSid && typeof _clearComposerDraft === 'function') "
|
||||
"_composerDraftClearPromise=_clearComposerDraft(activeSid);"
|
||||
"_composerDraftClearPromise=_clearComposerDraft(activeSid,_submittedDraftTextForClear,_submittedDraftFilesForClear);"
|
||||
)
|
||||
assert main_clear in MESSAGES_JS, "main send path must clear the persisted draft at send time"
|
||||
# The composer textarea wipe must sit immediately above that clear.
|
||||
clear_idx = MESSAGES_JS.find(main_clear)
|
||||
window_before = MESSAGES_JS[clear_idx - 700:clear_idx]
|
||||
assert "const _submittedDraftTextForClear=$('msg').value||'';" in window_before
|
||||
assert "const _submittedDraftFilesForClear=Array.isArray(_failedSendFilesSnapshot)?[..._failedSendFilesSnapshot]:[];" in window_before
|
||||
assert "$('msg').value='';autoResize();" in window_before, (
|
||||
"the main-path composer wipe must precede the persisted-draft clear"
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in a new issue