fix(chat): warn before regenerate, resend or edit deletes later messages; offer fork (#6225)

* fix(chat): warn before "regenerate" permanently deletes later messages

Both "regenerate" entry points — the per-message footer's ↻ "Regenerate
from here" button, and the vision editor's "Regenerate message" button —
call POST /api/session/{id}/truncate, which does a real, permanent SQL
delete of every message after the point clicked, then generates a new
reply. Clicking either on anything but the very last exchange did this with
zero warning: the button's own label doesn't say it also erases the rest
of the conversation, and neither button visually signals it's destructive
differently from any other message-footer action.

Caught live: a mis-click destroyed a real conversation with no way to
recover it (no backup covered the affected window). This adds a
confirmation prompt — via the existing uiModule.styledConfirm — whenever
the action would delete more than just the message(s) being regenerated,
stating the exact count of messages that would be lost. Regenerating the
most recent exchange (the everyday case, nothing exists after it) is
unaffected and still proceeds with zero friction.

New test: tests/test_regenerate_truncate_warning_js.py — drives the real
regenerateFrom/resendUserMessage functions under Node with a fake message
list DOM, confirming: the last-exchange case never prompts and truncates
directly; an earlier case prompts with the correct count; cancelling
aborts before any network call; confirming proceeds with the correct
keep_count. Also fixed a pre-existing, unrelated gap this surfaced while
verifying the full suite: test_pr6020_browser_review_regressions.py's
Node harness never stubbed `navigator` at all (sessions.js reads
navigator.platform at module load) — a gap on origin/dev itself, not
caused by this branch's diff, using the same Object.defineProperty fix
already applied on #6019/#6148 for the sibling issue there (Node 21+'s own
getter-only navigator global, not relevant to this specific failure but
the same defensive pattern).

Verified: full pytest suite in-container, 5798 passed / 11 pre-existing
unrelated failures (README/docs/agent-loop baseline) / 0 new.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* test(chat): cover plain resend after dev's replace-by-default change

dev made plain resend (no options) replace from the clicked turn by
default, with append-only resend opt-in via { append: true } (see
test_resend_message_nondestructive.py). The footer "Resend message"
button calls resendUserMessage(el) with no options, so it now truncates
everything after the clicked turn and goes through this PR's warning.
Replace the old "plain resend never prompts" test, which assumed the
previous append-by-default behavior, with one asserting that plain
resend on an earlier pair prompts with the right count, and one
asserting that explicit append-only resend never truncates or prompts.

Also resolve ui.js/sessions.js through the same cache-busting query
string chat.js imports them with. ES modules cache by full URL, so the
harness's unversioned import of ui.js was a second module instance and
its uiMod.styledConfirm stub never reached the instance chat.js calls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(chat): warn before an edit deletes later messages; offer to fork instead

Sending an edited user message truncates the chat at that message, the
same unrecoverable server-side delete as regenerate and resend, but did
it with no prompt. It now goes through the same confirmation, worded for
the action ("Sending this edit will also permanently delete the N
messages after it"). Cancelling leaves the editor open with the draft.

The confirmation for all three actions (regenerate, resend, edit) also
offers "Fork from here": it forks the chat just before the user turn,
switches to the new chat, and replays the action there (the same message,
or the edited text), leaving the original chat untouched. forkFrom now
shares the same fork helper.

Tests drive the real editUserMessage Send handler under Node and cover
the edit prompt (count, cancel, last-message no-prompt) and the fork
choice for each action (fork keep_count, no truncate, replayed text).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(chat): cut and fork a chat by message id, not loaded-page position

Regenerate and edit sent keep_count, the clicked message's position among
the messages loaded in the page. A long chat loads only its newest messages
until the user scrolls up, so that position is smaller than the real one and
the truncate deleted real messages above the clicked one (regenerating the
latest reply in a ~50-message chat removed about a dozen earlier messages).
Resend already sent before_msg_id; regenerate and edit now do the same.

The fork options this PR adds had the same flaw. Fork now accepts
before_msg_id (fork before that message) and through_msg_id (include it),
resolved against the stored messages; keep_count stays as the fallback for a
message that has no id yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(chat): keep a regenerated turn's message id current; stop on a refused cut

Regenerate keeps the original user bubble on screen and resubmits its text
without drawing a new bubble. The resubmitted message is a new DB row, but
the bubble kept the deleted row's id, so the next regenerate's cut was
refused (404) and the page sent the turn anyway, stacking a second reply.

The stream now reports the saved user message's id (user_message_saved),
which the page attaches to the user bubble, or to the bubble a regenerate
kept. Regenerate and edit stop with an error when the server refuses the
cut instead of sending.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(chat): name the buttons in words instead of symbols

CONTRIBUTING asks for no Unicode emoji in UI or code; the test comments
used the regenerate and pencil glyphs to name buttons.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(chat): share the fork-and-send step across edit, resend and regenerate

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(chat): pass the regenerate fork its attachment ids explicitly

Resend already hands its collected file ids to _forkAndSend; regenerate
relied on the earlier _pendingRegenAttachments assignment. Pass _regenIds
too so both fork paths read the same, and check the hand-off in the
regenerate tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(chat): assert fork choices keep the user message's attachments

Run regenerate and resend's "Fork from here" against a user message with
an uploaded file and check that the send fired in the fork carries its
file id, replacing the source-text check. chat.js exposes a read-only
_pendingRegenAttachmentIds() for this, like cookbook-hwfit.js's test hooks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(chat): preserve attachments across edit and regeneration

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Alexandre Teixeira <general@alexandreteixeira.dev>
This commit is contained in:
John W
2026-11-01 02:47:32 +00:00
committed by GitHub
co-authored by Claude Opus 5.5 Alexandre Teixeira
parent 285d05e72e
commit 5353241743
5 changed files with 769 additions and 50 deletions
+39
View File
@@ -90,3 +90,42 @@ def test_fork_does_not_corrupt_source_message_metadata(monkeypatch):
# ...and the source session's _db_id values are untouched.
assert source.history[0].metadata["_db_id"] == "src-0"
assert source.history[1].metadata["_db_id"] == "src-1"
def _fork_with(monkeypatch, body, db_ids):
"""Fork a 4-message source whose DB rows have the given ids."""
monkeypatch.setattr(mod, "_verify_session_owner", lambda *a, **k: None)
source = _FakeSession(name="Long chat")
for i, role in enumerate(["user", "assistant", "user", "assistant"]):
source.history.append(ChatMessage(role=role, content=f"m{i}", metadata={"_db_id": db_ids[i]}))
manager = _FakeSessionManager(source)
rows = [SimpleNamespace(id=i) for i in db_ids]
class _Query:
def filter(self, *a): return self
def order_by(self, *a): return self
def all(self): return rows
class _DB:
def query(self, *a): return _Query()
def close(self): pass
monkeypatch.setattr(mod, "SessionLocal", lambda: _DB())
router = mod.setup_history_routes(manager)
class _Req:
async def json(self): return body
asyncio.run(_fork_handler(router)(_Req(), "src-id"))
return [m.content for m in manager.created.history]
def test_fork_before_a_message_id_ignores_a_stale_keep_count(monkeypatch):
copied = _fork_with(monkeypatch, {"before_msg_id": "c", "keep_count": 0}, ["a", "b", "c", "d"])
assert copied == ["m0", "m1"]
def test_fork_through_a_message_id_includes_it(monkeypatch):
copied = _fork_with(monkeypatch, {"through_msg_id": "b", "keep_count": 0}, ["a", "b", "c", "d"])
assert copied == ["m0", "m1"]