Skip to content

append store: next_offset seeks to a raw byte offset and read_to_string fails on a split codepoint, wedging the journal permanently #108

Description

@oxoxDev

What happens

JsonlAppendStore::next_offset (src/harness/store/mod.rs:328-376) reads the tail of a journal file to find the append offset:

let start = len.saturating_sub(window);   // window = 4096
file.seek(SeekFrom::Start(start))?;
let mut buf = String::new();
file.read_to_string(&mut buf)             // <-- start is a raw byte offset

start is a raw byte offset with no reason to fall on a UTF-8 char boundary. serde_json does not escape non-ASCII, so model text sits in these files as raw multi-byte UTF-8. When len - 4096 lands mid-codepoint, read_to_string returns InvalidData("stream did not contain valid UTF-8"), wrapped at :355 into:

append store read error: stream did not contain valid UTF-8

Why it never recovers

next_offset runs before the write (:423 before :431). So the failure means nothing is appended, len never changes, the same byte keeps splitting the same codepoint, and every subsequent observation fails identically. The ? also returns before the window *= 4 growth that would otherwise move the start offset — so the one escape hatch in the loop is unreachable.

The file is not corrupt. The reader is wrong.

Evidence

Scanned 83,258 journal files on one machine: 79 are already in the wedged state, and zero contain invalid UTF-8. Downstream, a host consuming this store logged the error 643 times in a single session with the agent journal never being written.

Note on the doc comment

The code intends to tolerate a torn trailing line, and the line-level walk does handle it — complete_from (:359) discards the first partial line. But a truncated codepoint dies at read_to_string before line-splitting ever happens, so that guarantee is unsound for exactly the bytes the function already plans to throw away.

Suggested fix

Roughly five lines, either:

  • read the tail as bytes and back the start up to a char boundary before decoding, or
  • find the last \n within the raw window and decode from there.

Test coverage

src/harness/store/test.rs has no coverage of next_offset, the 4096 window, or non-ASCII payloads — which is why this has stayed invisible. A regression test wants a file whose len - 4096 deliberately lands mid-codepoint.

Related

Downstream report: tinyhumansai/opencompany#887. Note the separate log-spam issue (opencompany#450) is already resolved by the suppression worker added in #97 — this is a different defect that the old logging merely made loud.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: p1Next. Wrong behaviour a user will hit, or a security weakness behind a condition.

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions