Skip to content

fix(overleaf): resume sync steps a crash left unpublished - #624

Merged
yaacovcorcos merged 5 commits into
mainfrom
claude/overleaf-resumable-steps-20261022
Oct 22, 2026
Merged

yaacovcorcos merged 5 commits into
mainfrom
claude/overleaf-resumable-steps-20261022

Conversation

@yaacovcorcos

@yaacovcorcos yaacovcorcos commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What this fixes

Follow-up to #620 (Codex's round 5 there). A crash at certain narrow points left a valid group of changes partly applied and handed it to review, even though recovery had what it needed to finish. This has to land before step 3 lets the applier write to the project folder.

  1. A crash before a step's bytes were staged. Settlement closed the step as skipped, and the applier accepted that for good. Now the applier tries such a step again under a new record (step-N-1, …), using the bytes in plan.json, and reruns every check.
  2. A crash just before a native call. Recovery couldn't tell whether the call had run. Now it reads the evidence. The helper's lock is held by the process running the call until that call ends, so once recovery has the lock, what it sees is final.
Call Never ran when… Ran when…
exchange (edit) the staged copy is still in its slot the staged copy is at the target → goes on as displaced
capture (delete, rename, folder removal) nothing is in retained something is → goes on as displaced
create (new file) nothing is at the target and its folder's change time is unchanged since just before the call (parentChange) can't be shown; stays for review as before

When a call never ran, the step goes back to prepared and every check runs again.

Also fixed, found in review

  • Recovery never makes an epoch-checked publication. Generated files such as a PDF export are published only if the project hasn't changed since they were made, and only the original caller can check that. This also closes an older hole for a crash at prepared.
  • Folder removals belong to their group. A folder emptied before a file takes its place carries the group's checks, and its removal counts toward deciding whether the group was interrupted.
  • An incomplete staged copy is discarded. One whose identity was never recorded was still being written at the crash. It's private and was never published, so it's discarded and staged again; without the bytes, the step closes unpublished and the applier retries it.

Known limits

Both end in review with nothing lost.

  • A crash in the instant after a create's native call returns. The file at the target could be the new copy, or another writer's file with the same bytes. feat(documents): add visual LaTeX authoring and reusable templates #353's rule is that recovery trusts object identity, never same-content files, so it doesn't guess.
  • A crash between the file layer's intent index and a step's first journal. Folders a new file needs can be created in that window, so recovery can't prove nothing happened. The file waits in retained attention for explicit review, as feat(documents): add visual LaTeX authoring and reusable templates #353 designed. A round-2 attempt to settle this window was wrong (Codex round 3) and was reverted.

Tests

  • Real-crash tests. A child process kills itself at a native hook, or inside a file rename or open, covering:
    • every unit shape stopped at checked (just before the call) and at called (just after it);
    • a unit where every file changes, at every point, at the first and second step;
    • folder removals;
    • create evidence;
    • a crash while staging, and before the first journal.
  • Native-layer unit tests:
    • a capture that never ran is finished;
    • a fenced publication is never made on recovery.
  • Mutation checks: every fix was removed in turn, and its tests failed each time.

vp test run src/scient/overleaf src/scient/workspace src/workspace: 701 tests. Typecheck and lint are clean. The intermittent plainPublication lock-timing test recorded on #620 is unrelated. These tests need macOS native publication and are skipped on Linux CI.

Review

Four Codex rounds (gpt-6.1-sol, high reasoning), recorded in the comments.

  • Rounds 1–3 found 7 gaps. Six are fixed with tests; the seventh is the known limit above.
  • Round 4: merge-ready, no new findings (230 tests and 26 extra probes).

Plans: #535.

🤖 Generated with Claude Code

A crash before a step's bytes were staged let settlement close the step as
skipped, and the applier then left its group half-applied. The applier now
tries such a step again under a new record (step-N-1, ...) from plan.json.

A crash between recording `publishing` and the native call left recovery
unable to tell whether the call ran. The helper's lock outlives a dying
server only until its call retires, so recovery now reads what stands:
- exchange: staged copy still in its slot -> never ran; at the target -> ran
- capture: nothing in `retained` -> never ran; something there -> ran
- create: nothing at the target and its folder's change time unchanged since
  just before the call (new `parentChange`) -> never ran; otherwise review
A step that never ran goes back to `prepared` and every check runs again.

Adds a `called` test hook after each native call and real-crash tests for
every unit shape at `checked` and `called`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Oct 22, 2026
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.6 KiB 5.6 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 4.3 KiB 4.3 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 26.2 KiB 26.2 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.6 KiB 5.6 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 4.3 KiB 4.3 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 26.6 KiB 26.6 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 8e054c8 · PR result: 4ffb0ce · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Full compact HTTP snapshot with live cursor.

  • Codex decoded thread snapshot: 133.1 KiB
  • Claude decoded thread snapshot: 133.4 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

…ls in their group

Codex round 1 on #624:
- Recovery (a call without bytes) never makes a publication fenced by an
  expected mutation epoch: only its caller checks the epoch under the root
  lease. Such a record closes skipped with nothing published. This also closes
  the older case of a crash at `prepared`.
- A folder removed before a file takes its place now carries the step's group
  checks, and the applier counts those removals when deciding whether a group
  was interrupted.

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

Copy link
Copy Markdown
Contributor Author

Codex review, round 1 (gpt-6.1-sol, high reasoning)

Verdict on d19855a107: no, two P2s. Both were reproduced with real SIGKILLs and are now fixed in HEAD.

Finding Fix
Recovery skipped the epoch check on generated files. Some writes (a PDF export, for example) are published only if the project hasn't changed since they were produced. Settling a crashed one published it without that check. Recovery has no bytes and can't check the epoch, so it never makes a fenced publication; the record closes skipped with nothing published. The older crash at prepared had the same hole and is closed too. Test: never makes a fenced publication on recovery after prepared / checked.
Folder removals escaped their group. An emptied folder giving way to a file was removed without the group's checks, and wasn't counted when deciding whether the group was interrupted. The folder removal carries the step's group checks, and the applier counts its records. Tests: keeps a group whose file changed after a crash at the folder removal's checked / called.

Each fix fails its tests without it (mutation-checked: 2, 1 and 1 failures).

What held:

  • The lock handoff: in-flight helper commands finish before recovery can act.
  • Resumed steps recheck everything.
  • Same-content creates stay unadopted.
  • Retries keep old receipts.
  • A 20,000-cycle folder change-time probe found no repeats.

Now: 697 tests pass. Typecheck and lint are clean. Round 2 is reviewing the new head.

🤖 Generated with Claude Code

…taging

Codex round 2 on #624:
- An admission indexed just before a crash, with no journal and nothing kept,
  is durably refused by settlement (nothing can have happened), so it no longer
  holds its file in attention; the applier retries such a step under a new
  record.
- A staged copy whose identity was never recorded was still being written:
  it is private and unpublished, so it is discarded and staged again, or the
  step closes unpublished when recovery has no bytes.

The crash-test child can now die inside a file rename or open, reaching
windows between the native hooks.

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

Copy link
Copy Markdown
Contributor Author

Codex review, round 2 (gpt-6.1-sol, high reasoning)

Verdict on 8d722ea208: no, two P2s. Both round-1 fixes held. Both new gaps were reproduced with real SIGKILLs at points between the native hooks, and both are now fixed in HEAD.

Finding Fix
A crash just before a step's first journal is renamed into place. The admission was indexed but has no journal, so recovery reused the attempt and failed, and the file was held in attention. Settlement durably refuses an indexed admission that has no journal and keeps nothing. That journal comes before any staging or native call, so nothing can have happened. The applier then retries the step under a new record.
A crash while the staged copy was still being written. The partial copy's wrong hash sent the step to attention for good. A staged copy whose identity was never recorded is private and unpublished. It's discarded and staged again; without the bytes, the step closes unpublished and the applier retries it.

The crash-test child can now also die inside a file rename or open. Test: finishes a unit after a crash in step 0/1's rename/open, 4 cases. Each fix fails its tests without it (mutation-checked: 2 failures each).

What held:

  • Fenced recovery refuses publication, while the caller's own replay still publishes.
  • Folder-removal guards and accounting.
  • Repeated crashes and attempt numbering.

Now: 701 tests pass. Typecheck and lint are clean. Round 3 is reviewing the new head.

🤖 Generated with Claude Code

…ve layer decides

Codex round 3 on #624 showed that settling an indexed admission without a
journal as "nothing happened" was wrong: parent folders can be created before
the first journal, settlement does not own the identity index it wrote to, and
prune identities cannot be retried. That window is reverted to the native
layer's design: the file stays in retained attention for explicit review, and
nothing is lost. The applier's matching no-journal retry is removed with it.

The crash test now asserts that outcome precisely, and still proves that a
crash while staging finishes the group.

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

Copy link
Copy Markdown
Contributor Author

Codex review, round 3 (gpt-6.1-sol, high reasoning)

Verdict on d633fe9b78: no, three P2s. All three come from one round-2 change: settlement treating an indexed admission with no journal (and nothing kept) as "nothing happened" and refusing it durably. That premise was wrong:

  1. Folder removals reuse fixed identities, so a refused one could never be retried.
  2. Settlement doesn't hold the lock on the identity index it wrote to. A matching retry could pass preflight in between and delete a file under an identity marked "unchanged".
  3. Folders a new file needs are created before its first journal, so "no journal" doesn't prove nothing changed. Settlement then erased that directory-preparation attention.

Resolution: reverted, not patched further. The window between the file layer's intent index and a step's first journal can't be resolved from evidence. #353's native layer already handles it as retained attention: the file waits for explicit review, and nothing is lost. The applier's matching "no journal" retry is removed too.

Test: holds a file for review after a crash before step 0/1's journal asserts WorkspaceFileRecoveryRequiredError with retained-attention for the right file, and that no bytes changed beyond the steps already published.

Kept from round 2: discarding a staged copy whose identity was never recorded. Codex's live-writer probes showed the record-folder lease protects it. Test: finishes a unit after a crash while staging step 0/1.

Known limits of this PR, all ending in review with no data lost:

  • A crash in the instant after a create's native call returns.
  • A crash between the file layer's intent index and a step's first journal.

Now: 700 of 701 tests pass. The one failure is the intermittent plainPublication lock-timing test recorded on #620 as a pre-existing race in the plain publication path; it passed on rerun. Typecheck and lint are clean. Round 4 is reviewing the new head.

🤖 Generated with Claude Code

@yaacovcorcos

Copy link
Copy Markdown
Contributor Author

Codex review, round 4 (gpt-6.1-sol, high reasoning)

Verdict on e8cf4ff105: merge-ready, with no new findings, within the accepted review limits.

What held:

  • The revert is complete. Settlement matches main except for the folder-removal group checks, and no refusal is written. The test asserts retained-attention for the right file.
  • Slot recovery:
    • A partly written, unrecorded staged copy is discarded.
    • Recovery without bytes closes the step skipped.
    • A retry from the plan finishes the group.
    • A changed group stops publication.
    • Repeated crashes recover.
  • The earlier fixes: evidence from native publication, the epoch fence, folder-removal guards and accounting, and the cautious create-after-call attention.

Evidence: 230 committed tests and 26 extra probes passed on macOS.

🤖 Generated with Claude Code

@yaacovcorcos
yaacovcorcos marked this pull request as ready for review October 22, 2026 19:30
@yaacovcorcos
yaacovcorcos added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit 9d312f3 Oct 22, 2026
91 of 96 checks passed
@yaacovcorcos
yaacovcorcos deleted the claude/overleaf-resumable-steps-20261022 branch October 22, 2026 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant