Skip to content

fix(claude-native): stop the resume seek from skipping a just-injected prompt - #4403

Merged
daniellok-db merged 1 commit into
mainfrom
fix-skipped-message
Aug 8, 2026
Merged

fix(claude-native): stop the resume seek from skipping a just-injected prompt#4403
daniellok-db merged 1 commit into
mainfrom
fix-skipped-message

Conversation

@daniellok-db

Copy link
Copy Markdown
Contributor

Related issue

Closes #

Summary

A prompt sent to a resuming claude-native session sometimes never reached the
Omnigent DB — while still showing in Claude's TUI pane. No error, no warning,
nothing in the runner log.

Commit 1 adds diagnostics for the three silent skip paths. It caught the bug on
the first run:

12:13:27.580  synthesized prefix finished writing  ->  ends at byte 5920
12:13:27.659  tmux launch returns, Claude starts
12:13:27.663  forwarder task created (fire-and-forget)
12:13:28.758  background turn starts, executor injects the prompt
12:13:29.037  Claude writes the prompt at byte 7757
12:13:29.989  forwarder seeds: byte_offset=24711 skipped_records=21   <-- the bug

start_at_end=True means "skip the prefix I just wrote" — it is set iff this
launch synthesized a resume transcript. But it was implemented as "skip
whatever exists when I get around to looking,"
and those are different things.

Seeding needs transcript_path from Claude's first hook; inject_user_message
waits on the same boot. The two are unordered, so the paste routinely wins.
Everything Claude wrote in that ~2.3s window then sits behind the cursor and
is skipped for the session's lifetime.

The record count is the proof. The synthesized prefix is 14 records (bytes
0–5920, all v5 UUIDs — the ones we generate). The seed skipped 21. The extra
7 are live Claude records, and one of them is the user's message.

So the skip was doing two different things at once:

bytes what verdict
0–5920 synthesized prefix, rebuilt from committed Omnigent items intended — forwarding it would duplicate history
5920–24711 whatever Claude wrote during boot the bug — the user's prompt included

The fix

The prefix length is already known before launch — all three synthesizing
paths return the path they wrote (_ensure_local_claude_resume_transcript on
cold resume, _clone_claude_transcript for a same-host fork, the items-rebuild
for a cross-family fork). So measure it there and pass start_at_offset
through, instead of relying on a later stat:

_ensure_local_claude_resume_transcript() -> target
  _measured_prefix_bytes(target)          # 5920, at 12:13:27.580 — before launch
    -> supervise_forwarder(start_at_offset=5920)
      -> _ensure_state_for_transcript: byte_offset = 5920

The skip becomes exactly the prefix regardless of when the forwarder is
scheduled, so the race is removed rather than narrowed.

ELI5

We rebuild the conversation history into a file, hand it to Claude, and tell the
tailer "start reading after the part I wrote." It was measuring "the part I
wrote" after Claude had already started appending to the same file — so it
skipped the user's new message along with the history. Now we measure the file
before Claude touches it.

Kept deliberately

  • start_at_end stays for reattach (claude_native.py:2685), where nothing
    was synthesized and a live end-offset is the right answer. The CLI attach
    path has no concurrent inject, so it is unaffected.
  • Clamped to the transcript end, so a truncated/replaced file cannot leave
    the cursor past EOF (where every later read looks like a stale-cursor reset).
  • A failed measurement falls back to the old behaviour, not to 0
    re-forwarding all of history would be the worse failure.
  • The seed log now reports source=measured_prefix|transcript_end, so a future
    divergence between the skipped-record count and the prefix is visible.

Other harnesses

claude-native only. supervise_forwarder here is a distinct function from
the same-named codex one (no shared code), and no other harness forwarder has
start_at_end at all — goose / hermes / codex / cursor / kimi all report 0
references. _ensure_state_for_transcript is claude-only.

Test Plan

Branched from latest main (f9ec924a) — this is a pre-existing bug, not a
regression from #4344, so the fix is scoped away from that PR.

pytest tests/test_claude_native_forwarder.py tests/test_claude_native_bridge.py
# -> 363 passed, 1 pre-existing failure (see below)
pytest tests/runner/

Verified against the real transcript of the session that lost a message:

path seeds at items forwarded lost prompt recovered
old (start_at_end) 30365 0 no
new (start_at_offset=5920) 5920 2 yes

New coverage:

  • test_measured_prefix_seed_keeps_a_prompt_injected_during_boot — replays the
    exact race (prefix written, Claude appends a prompt, then we seed).
    Confirmed it fails without the fix (cursor at 305/EOF instead of
    186/prefix), so it genuinely guards the regression.
  • test_measured_prefix_never_seeks_past_the_transcript_end — the clamp guard.
  • Commit 1's three diagnostic tests, also confirmed failing without their
    production change
    (4 failed / 1 passed when reverted).

Pre-commit: ruff format + ruff check pass, zero pyrefly errors in the changed
line ranges (the 9 reported are missing-import in untouched files — optional
deps absent from this worktree's .venv, present in CI).

Manual verification

# Resume a claude-native session and send a message immediately.
# The message must appear in the chat transcript, not just the terminal pane.
grep "cursor seeded past existing records" ~/.omnigent/logs/runner/runner-*.log
#   before: skipped_records=21  (14 prefix + 7 live)
#   after:  skipped_records=14  source=measured_prefix

The record count is the tell: it should now equal the synthesized prefix
exactly.

Demo

N/A — no visual change. The fix is that a message stops disappearing.

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

The race itself is covered deterministically by writing the prefix, appending a
record, and then seeding — which is the real ordering, without needing to
schedule a real Claude boot against a real tmux pane. Both new tests were
verified to fail with the production change reverted.

One pre-existing failure, unrelated and present on clean main:
test_claude_native_bridge.py::test_relay_close_keeps_advertisement_owned_by_newer_relay.
Also pre-existing on clean main in this shell: 4 test_claude_native.py
provider-config tests that read ambient ANTHROPIC_* env.

Changelog

Resuming a claude-native session no longer drops a message sent right after the
session starts.

@github-actions github-actions Bot added the size/L Pull request size: L label Aug 8, 2026
@omnigent-ci

omnigent-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. The core fix is correct and the change is well-contained:

  • start_at_offset is threaded correctly from the three synthesizing paths (_ensure_local_claude_resume_transcript, _clone_claude_transcript, cross-family rebuild) through supervise_forwarderforward_claude_transcript_to_session_ensure_state_for_transcript, and consulted with the right precedence (over start_at_end).
  • The clamp min(start_at_offset, _transcript_end_offset(...)) is sound: the measured st_size is a raw file size, but clamping to the record-aligned end offset means the cursor never lands past EOF and never lands on a partial trailing record, so the fingerprint stays coherent. The test_measured_prefix_never_seeks_past_the_transcript_end case exercises this.
  • The failure fallback is correct — _measured_prefix_bytes returns None on OSError, which leaves start_at_offset=None and preserves the prior start_at_end behaviour (live end-offset) rather than collapsing to 0 and re-forwarding all history.
  • start_at_end is correctly retained for the reattach path, where nothing was synthesized and a live end-offset is the right seed.
  • Tests genuinely reproduce the race (test_measured_prefix_seed_keeps_a_prompt_injected_during_boot appends a "boot-window" record after measuring the prefix, then asserts the injected prompt is still forwarded), rather than just asserting the new plumbing exists.

Security vulnerabilities

None. No new external input, deserialization, path handling, or auth surface. The added _jsonl_record_count reads a local transcript with bounded chunked reads and swallows OSError to 0; it's diagnostic-only and cannot affect forwarding decisions.

Non-blocking notes

  • Blocking I/O off the thread pool. In the new start_at_offset branch, _transcript_end_offset(transcript_path) is called synchronously inside the coroutine, whereas the sibling start_at_end branch uses await asyncio.to_thread(...). The read is tiny (seek to EOF), so impact is negligible, but wrapping it in to_thread for consistency would keep the event loop clean.
  • Measured-file / hook-file identity assumption. resume_prefix_bytes is measured against the file the launch synthesized; the forwarder seeds against whatever transcript_path Claude's first hook advertises. The fix relies on those being the same file (true for --resume into the same session id). If they ever diverged, the prefix length would be applied to the wrong file and the clamp could under-skip (duplicating history). This is the same assumption the prior start_at_end path already made, so it's not a regression — just worth a comment noting the invariant.
  • The two extra diagnostic log paths (unrecognized commandMode, and "consumed with no items parsed") are at INFO; the "consumed with no items" path is described as "usually benign" and fires on common scaffolding/meta records, so keep an eye on log volume — it could be chatty on active sessions.

Summary

A focused, correct fix for a real silent data-loss bug: the resume seek was measuring the prefix after Claude had begun appending, skipping the freshly-injected user prompt for the session's lifetime. Measuring the prefix before launch and passing an explicit start_at_offset removes the race rather than narrowing it, with a sensible clamp and a safe fallback. Diagnostics are proportionate and the tests actually reproduce the failure. As a backend forwarder/logging fix with no user-visible surface, no visual demonstration is needed. Ready to merge pending the minor consistency nits above.


Automated review by Polly · workflow run

Comment thread omnigent/claude_native_forwarder.py Fixed
…me prefix

A prompt sent to a resuming claude-native session sometimes never reached the
Omnigent DB while still showing in Claude's TUI pane — no error, no warning.

`start_at_end=True` means "skip the prefix I just wrote" — it is set iff this
launch synthesized a resume transcript from committed Omnigent history (which
the DB already has, so forwarding it would duplicate the conversation). But it
was implemented as "skip whatever exists when I get around to looking", and
those are different things. Seeding requires `transcript_path` from Claude's
first hook, and `inject_user_message` waits on the same boot; the two are
unordered, so the paste routinely wins. Everything Claude wrote in that
window — the user's prompt included — then sat behind the cursor, skipped for
the session's lifetime.

The prefix length is already known before launch: all three synthesizing paths
(`_ensure_local_claude_resume_transcript` on cold resume, `_clone_claude_transcript`
for a same-host fork, the items-rebuild for a cross-family fork) return the path
they wrote. Measure it there and pass `start_at_offset` through instead of
relying on a later `stat`. The skip becomes exactly the prefix regardless of
when the forwarder is scheduled, so the race is removed rather than narrowed.

`start_at_end` stays for reattach, where nothing was synthesized and a live
end-offset is correct — the CLI attach path has no concurrent inject. The
offset is clamped to the transcript end so a truncated/replaced file cannot
leave the cursor past EOF, and a failed measurement falls back to the old
behaviour rather than to 0 (re-forwarding all history is the worse failure).

claude-native only: `supervise_forwarder` here is distinct from the same-named
codex function, and no other harness forwarder has `start_at_end`.

Co-authored-by: Isaac
@github-actions github-actions Bot added size/M Pull request size: M and removed size/L Pull request size: L labels Aug 8, 2026
@daniellok-db

Copy link
Copy Markdown
Contributor Author

/review

@omnigent-ci

omnigent-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. The fix is correct and well-scoped.

I verified the logic against the source:

  • In _ensure_state_for_transcript, the new start_at_offset branch is checked before start_at_end and computes byte_offset = min(start_at_offset, end_offset) where end_offset = _transcript_end_offset(...) (last complete record). This makes the skip exactly the synthesized prefix regardless of what Claude appended during boot, which is the race being fixed. The clamp against the complete-record end also guards against a truncated/replaced file leaving the cursor past EOF.
  • The persisted-cursor early returns (state/disk_state matching transcript_path) still take precedence, so a relaunch resumes from its saved cursor rather than re-seeding — correct.
  • All three synthesizing paths in _auto_create_claude_terminal (_ensure_local_claude_resume_transcript, _clone_claude_transcript, cross-family items-rebuild) set resume_prefix_bytes from the file they just wrote, and the single supervise_forwarder(..., start_at_offset=resume_prefix_bytes) call threads it through. The reattach path leaves it None, so start_at_end still governs there — matching the PR's stated intent.
  • line_cursor=0 with a nonzero byte_offset matches the existing start_at_end convention, so no new inconsistency is introduced.
  • Failure fallback is correct: _measured_prefix_bytes returns None on OSError, which reverts to the prior start_at_end behavior rather than seeding at 0.

Security vulnerabilities

None. No new inputs, network calls, deserialization, or path handling — the change only moves a byte-offset measurement earlier in the launch sequence.

Non-blocking notes

  • _measured_prefix_bytes uses stat().st_size (raw file size) while the seed clamps against _transcript_end_offset (offset after the last complete record). If a synthesized prefix ever lacked a trailing newline, st_size would exceed the complete-record boundary and the min(...) clamp would pull it back to the last complete record — safe, but it means the "measured prefix" and the actual seed could differ silently. Since the synthesizers write newline-terminated JSONL these are equal in practice; worth a one-line assertion or using _complete_jsonl_end_offset at measurement time for symmetry.
  • Edge case (not a regression): if a resume silently fails and Claude opens a different, fresh transcript path than the one measured, the measured start_at_offset would be applied to the new file and min(...) could skip its early content. This is the same failure mode the old start_at_end had (it would also seek to that file's end), so behavior is unchanged — just flagging that this path isn't defended by the new logic.
  • The source=measured_prefix|transcript_end seed log the description mentions is a good observability add; confirm it actually distinguishes the two branches in the emitted log so a future divergence between skipped-record count and prefix stays visible.

Summary

A tight, well-reasoned fix that removes a genuine boot-time race rather than narrowing it: the resume-prefix length is now measured before Claude launches and seeded verbatim (clamped to the transcript end), so a prompt injected during Claude's boot window is no longer stranded behind the forward cursor. The change is claude-native-only, correctly preserves start_at_end for reattach, falls back safely on measurement failure, and ships a focused regression test plus a clamp test. No blocking or security concerns. This is purely internal transcript-forwarding plumbing with no user-visible rendering change, so no visual demonstration is required — the regression test and log traces adequately evidence the fix.


Automated review by Polly · workflow run

@daniellok-db
daniellok-db merged commit de8aee8 into main Aug 8, 2026
66 checks passed
@daniellok-db
daniellok-db deleted the fix-skipped-message branch August 8, 2026 13:13
@github-actions github-actions Bot added the no-doc-update Merged PR does not need a docs update label Aug 8, 2026
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: no-doc-update

Internal bugfix to the Claude native transcript forwarder's cursor-seeding logic (adding a measured-prefix offset to avoid skipping boot-window messages); no user-facing surface, integration, or documented behavior changed.

Auto-classified on merge. Set the label manually before merging to override. · run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-doc-update Merged PR does not need a docs update size/M Pull request size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants