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.

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