harden #5471: dual-signature suppression (Opus root fix) — covers stale prefix + attachment
Opus found the exact-match signature fails open in the COMMON case: the server's last persisted draft is often a PREFIX of the submitted text (Enter-to-send within 400ms cancels the pending debounced save), so matching only the submitted text misses it and the prefix restores (the literal 'tail of the already-submitted message' symptom). Root fix (Opus-recommended): capture BOTH signatures at send — the remembered SERVER-draft signature (_composerDraftPayloadSignatureForSid, read before the reset) AND the submitted-payload signature — and suppress a restore matching EITHER. The prefix IS the remembered server draft; the remembered files went through the same lossy serialization so they self-match; a genuinely new cross-tab draft matches neither and restores. Subsumes the file-canonicalization fix (kept as belt-and-suspenders). Suppression stores signatures[] not signature. Regression tests: stale-prefix suppressed (Opus Finding 1), file round-trip self-matches (Codex Finding 2), cross-tab-different still restores. Test harness updated to model send-time server draft vs later restore draft as distinct. Co-authored-by: ruizanthony <ruizanthony@users.noreply.github.com>
This commit is contained in:
1 parent
5a8dc91bbb
commit
6baf501dce
2 files changed
+61
-13
No files matched your search
+24
-11
@@ -86,17 +86,28 @@ function _composerDraftPayloadSignatureForSid(sid) {
|
|||||||
function _suppressComposerDraftRestoreAfterSubmit(sid, text, files) {
|
function _suppressComposerDraftRestoreAfterSubmit(sid, text, files) {
|
||||||
if (!sid) return;
|
if (!sid) return;
|
||||||
const previous = _composerDraftRestoreSuppressedUntilBySid.get(sid);
|
const previous = _composerDraftRestoreSuppressedUntilBySid.get(sid);
|
||||||
let signature = null;
|
// 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) {
|
if (arguments.length >= 2) {
|
||||||
signature = _composerDraftPayloadSignature(text, files);
|
_addSig(_composerDraftPayloadSignature(text, files)); // submitted payload
|
||||||
} else if (previous && typeof previous === 'object') {
|
} else if (previous && typeof previous === 'object' && Array.isArray(previous.signatures)) {
|
||||||
signature = previous.signature || null;
|
previous.signatures.forEach(_addSig);
|
||||||
} else {
|
|
||||||
signature = _composerDraftPayloadSignatureForSid(sid);
|
|
||||||
}
|
}
|
||||||
_composerDraftRestoreSuppressedUntilBySid.set(
|
_composerDraftRestoreSuppressedUntilBySid.set(
|
||||||
sid,
|
sid,
|
||||||
{ until: Date.now() + _COMPOSER_DRAFT_RESTORE_SUPPRESS_MS, signature },
|
{ until: Date.now() + _COMPOSER_DRAFT_RESTORE_SUPPRESS_MS, signatures },
|
||||||
);
|
);
|
||||||
// Local state must reflect the submitted/cleared composer immediately. The
|
// Local state must reflect the submitted/cleared composer immediately. The
|
||||||
// POST that clears the server-side draft is async; same-session refreshes can
|
// POST that clears the server-side draft is async; same-session refreshes can
|
||||||
@@ -119,11 +130,13 @@ function _isComposerDraftRestoreSuppressed(sid, text, files) {
|
|||||||
_composerDraftRestoreSuppressedUntilBySid.delete(sid);
|
_composerDraftRestoreSuppressedUntilBySid.delete(sid);
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
const signature = (suppression && typeof suppression === 'object') ? suppression.signature : null;
|
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
|
// Legacy/unknown callers still fail closed for the current TTL, but all send
|
||||||
// paths now pass a payload signature so a different cross-tab draft can restore.
|
// paths now pass payload signatures so a different cross-tab draft can restore.
|
||||||
if (!signature) return true;
|
if (!signatures || !signatures.length) return true;
|
||||||
if (_composerDraftPayloadSignature(text, files) === signature) return true;
|
if (signatures.indexOf(_composerDraftPayloadSignature(text, files)) !== -1) return true;
|
||||||
_composerDraftRestoreSuppressedUntilBySid.delete(sid);
|
_composerDraftRestoreSuppressedUntilBySid.delete(sid);
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -44,7 +44,7 @@ def _draft_restore_suppression_block() -> str:
|
|||||||
return SESSIONS_JS[start:end]
|
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) -> dict:
|
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:
|
if NODE is None:
|
||||||
pytest.skip("node not on PATH")
|
pytest.skip("node not on PATH")
|
||||||
restore_fn = _function_block(SESSIONS_JS, "function _restoreComposerDraft(draft, targetSid, opts={}) {")
|
restore_fn = _function_block(SESSIONS_JS, "function _restoreComposerDraft(draft, targetSid, opts={}) {")
|
||||||
@@ -60,6 +60,12 @@ def _run_case(*, initial_text: str, draft: dict | None, opts: dict | None, curre
|
|||||||
suppress_call = "_suppressComposerDraftRestoreAfterSubmit(sid);"
|
suppress_call = "_suppressComposerDraftRestoreAfterSubmit(sid);"
|
||||||
else:
|
else:
|
||||||
suppress_call = ""
|
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"""
|
script = f"""
|
||||||
const state = {{
|
const state = {{
|
||||||
value: {json.dumps(initial_text)},
|
value: {json.dumps(initial_text)},
|
||||||
@@ -80,7 +86,7 @@ const sid = 'boot-session';
|
|||||||
const S = {{
|
const S = {{
|
||||||
session: {{
|
session: {{
|
||||||
session_id: sid,
|
session_id: sid,
|
||||||
composer_draft: {json.dumps(draft)},
|
composer_draft: {json.dumps(send_time_draft)},
|
||||||
}},
|
}},
|
||||||
}};
|
}};
|
||||||
const currentSid = {json.dumps(current_sid)};
|
const currentSid = {json.dumps(current_sid)};
|
||||||
@@ -91,9 +97,15 @@ function _composerDraftHasPayload(text, files) {{
|
|||||||
}}
|
}}
|
||||||
function _rememberComposerDraftPayloadState(sid, text, files) {{
|
function _rememberComposerDraftPayloadState(sid, text, files) {{
|
||||||
state.rememberedDraft = {{sid, text, files}};
|
state.rememberedDraft = {{sid, text, files}};
|
||||||
|
if (S.session && S.session.session_id === sid) {{
|
||||||
|
S.session.composer_draft = {{text, files}};
|
||||||
|
}}
|
||||||
}}
|
}}
|
||||||
{suppression_block}
|
{suppression_block}
|
||||||
{suppress_call}
|
{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}
|
{restore_fn}
|
||||||
{draft_block}
|
{draft_block}
|
||||||
process.stdout.write(JSON.stringify({{
|
process.stdout.write(JSON.stringify({{
|
||||||
@@ -163,6 +175,7 @@ def test_same_session_submitted_clear_blocks_stale_server_draft_restore():
|
|||||||
opts={"preserveActiveInput": True},
|
opts={"preserveActiveInput": True},
|
||||||
current_sid="boot-session",
|
current_sid="boot-session",
|
||||||
force_reload=True,
|
force_reload=True,
|
||||||
|
remembered_server_draft={"text": "old submitted suffix", "files": []},
|
||||||
suppress_restore={"text": "old submitted suffix", "files": []},
|
suppress_restore={"text": "old submitted suffix", "files": []},
|
||||||
)
|
)
|
||||||
assert data["value"] == ""
|
assert data["value"] == ""
|
||||||
@@ -170,6 +183,27 @@ def test_same_session_submitted_clear_blocks_stale_server_draft_restore():
|
|||||||
assert data["updateSendBtnCount"] == 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():
|
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."""
|
"""A different same-session draft saved by another tab must not be hidden by send-time suppression."""
|
||||||
data = _run_case(
|
data = _run_case(
|
||||||
@@ -178,6 +212,7 @@ def test_same_session_submitted_clear_allows_different_cross_tab_draft_restore()
|
|||||||
opts={"preserveActiveInput": True},
|
opts={"preserveActiveInput": True},
|
||||||
current_sid="boot-session",
|
current_sid="boot-session",
|
||||||
force_reload=True,
|
force_reload=True,
|
||||||
|
remembered_server_draft={"text": "old submitted suffix", "files": []},
|
||||||
suppress_restore={"text": "old submitted suffix", "files": []},
|
suppress_restore={"text": "old submitted suffix", "files": []},
|
||||||
)
|
)
|
||||||
assert data["value"] == "new cross-tab draft"
|
assert data["value"] == "new cross-tab draft"
|
||||||
|
|||||||
Reference in new issue
Block a user