Skip to content

#177's resume adoption accepts an EMPTY journal, turning run_not_found into an internal error #185

Description

@kjgbot

A regression I introduced in #177, surfaced by the diagnostics from #175 and #176 on run 33982088411.

What CI showed

agent::rung_c_sigkill_boundaries_resume_only_unfinished_steps_via_real_cli failed in 63s (not hung — #175 bounded the read), and #176's dump named it:

before-first: no step.dispatch after resume: timed out after 60s waiting for a
protocol frame; the daemon sent nothing (see #174)
--- resume child ---
stdout (0 bytes):

stderr (114 bytes):
Error: journal_write_failed: read run spec: SQLite journal failed: Query returned no rows

--- journal (0 entries) ---

Zero journal entries. The SIGKILL landed after SqliteJournal::create but before the RunSpawned append.

Why #177 makes this worse

#177 taught run.resume to adopt an orphaned journal. Its gate is:

let adopted = match relayflowd_journal::SqliteJournal::open(&path) {
    Ok(journal) => journal.run_id() == params.run_id,
    Err(_) => false,
};

That checks the file opens and claims to be this run. It does not check the journal contains anything. An empty journal still has a meta row carrying the run id, so it opens, the id matches, it is adopted and registered — and then the resume path tries to read the run spec, finds no entries, and dies with:

journal_write_failed: read run spec: ... Query returned no rows

Before #177 this case returned a clean run_not_found. So the change traded a correct, legible refusal for a confusing internal error in the one sub-case it did not anticipate.

The distinction #177 missed

There are two kinds of orphan, not one:

state resumable? before #177 after #177
journal with RunSpawned, no registry row yes run_not_found (the bug) adopted ✓
journal created, killed before RunSpawned no run_not_found ✓ adopted, then internal error ✗

Only the first is a run. The second is a file that never became one.

Fix

Adoption should require the journal to carry a spawn record, not merely to open and match ids — if there is no spec to resume, it is not a run, and run_not_found is the honest answer. That also keeps #177's own orphan-file test meaningful: it asserts a truncated file is refused, and an empty-but-valid journal is the same class of thing.

Not fixing in this PR (#184 is SDK-test-only and this failure is unrelated to it).

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions