Skip to content

Move a part only after its handoff is in the fresh Session - #743

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:fix/parts-move-failure
Oct 22, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:fix/parts-move-failure

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Move a part only after its handoff is in the fresh Session
Fixes #740
parts.js moveOn flipped the Bot's pointer (sessionId / room.sessions, the new segment, the question card) and saved before it wrote the handoff package to the fresh Session, then set job.moved = true before sending it. Two failure windows lost work:

  • a rejected rt.save() left the in-memory state on the fresh Session, and the finalizer (seeing job.moved === false) forwarded the carried input to currentOf — now the fresh part — with no package, no transcript and no in-place checkpoint;
  • a failure after job.moved = true (ensureCapabilities, resolving the fresh Session, the package flush) made the finalizer return: the fresh part stayed current and empty and the stopped turn's input plus the old inbox messages were dropped.
    Also switchBeforeStep returned { kind: 'reject' } even when startSwitch returned early (no place, another switch, read-only), though DSH had already taken the step's messages out of the inbox.
    Now the handoff package is written to the fresh Session first; only then are the pointer/segment saved, with the previous state restored (and the fresh Session archived) if the save fails. The move is marked done only after the carried input and the old inbox messages were forwarded, so a failure lets the existing finalizer hand them on to the part that is current. startSwitch reports whether it started, and switchBeforeStep lets the step continue when it did not.
    core/test/work-dir.test.js fails the fresh Session's handoff flush, runs an idle folder move, and asserts the old part stays current with no segment recorded — it fails without the change (the Bot points at the empty fresh part). The existing switch/folder tests are unchanged. docs/subsystems/core-host.md records the order.
    Ran at the tip: npm run verify (green, TZ=UTC), ARCH_DOCS_BASE=upstream/main node core/scripts/check-arch-docs.mjs, ds-bot-patches generate + check. Not run: smoke (this macOS box has no setsid, so dev/bench.sh cannot start dsh web), Desktop, real-model steward.

moveOn flipped the Bot's Session pointer and saved before it wrote the
handoff package. A save failure then left the state on a fresh part with
no package, and a later failure (capabilities, resolving the fresh
Session, the package flush) left it there empty while the finalizer,
seeing job.moved, dropped the carried input. Send the package first, then
flip and save (restoring the old state if the save fails), and mark the
move done only after the input is forwarded. A switch that never starts
now lets the step continue instead of rejecting its messages.

Fixes ThinkFlowLab#740
@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at fd02696
Written-by: Reviewer

The reorder is right and the test pins it: the handoff package is written to the fresh Session before anything points at it, the pointer/segment save comes after with the previous state restored on failure, and job.moved is set only once the carried input and the old inbox messages were forwarded — so the existing finalizer no longer drops them. I read both rollback branches (chat and room) against the mutations they undo; each restores what the code touched (bot.sessionId + bot.segments + state.questions, or room.segments[bot.id] + room.sessions), archives the fresh Session, and leaves the switch map cleaned by the .finally. The !started → next() fix is right too: switchBeforeStep no longer rejects a step whose messages DSH already took out of the inbox when no switch began.

Ran:

  • cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/work-dir.test.js → 13/13 pass at this tip
  • red: the handoff-failure throw swallowed (the old behaviour, the move goes on anyway) → 12 pass / 1 fail, exactly a part move that fails before the handoff keeps the old part current (the old part stays current)
  • read, not run: the generated patches/ds-bot/* are covered only by CI here — node scripts/ds-bot-patches.mjs check needs git, which this box does not have

Not blocking, worth a follow-up issue rather than another tip:

  1. The save-failure rollback is the one new path with no test. The flush-failure case is covered, but if rt.save() throws, the restore of bot.sessionId/segments/room pointer is what keeps the Bot off an archived part, and nothing exercises it. If the harness can stub a save to throw once, a four-line case in work-dir.test.js would hold it.
  2. The reorder is only tested on a chat move (update-bot workDir). A room switch (room.sessions[bot.id] + room.segments) takes the other branch of the same code; the existing room cases do not fail the flush, so that branch's ordering is unheld. Same kind of case, with a room.

Neither changes my verdict: the reproduced defect is fixed, and the failure it was about is now pinned.

@Birfy
Birfy added this pull request to the merge queue Oct 22, 2026
Merged via the queue into ThinkFlowLab:main with commit d5fefe0 Oct 22, 2026
9 checks passed
HugoXK added a commit to HugoXK/FlyBot that referenced this pull request Oct 22, 2026
ThinkFlowLab#743 wrote the handoff package before the state flip so a failed handoff
would keep the old part current. The fresh part's handoff followup starts
a turn, and its events can arrive before the new segment is saved; a
second switch then reads partsOf without the previous segment and loses
its note (the full core suite showed it deterministically under load).
Save the new segment first, then write the package; a failed package
rolls the pointer back (best effort saved) and archives the fresh
Session, so the finalizer still hands the input on in the old part.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A part move that fails after the state change leaves an empty fresh part and drops input

2 participants