Skip to content

feat(chat): merge a forked session back into its parent - #4904

Open
jeeshofone wants to merge 1 commit into
kirodotdev:mainfrom
jeeshofone:feat/fork-merge-back
Open

feat(chat): merge a forked session back into its parent#4904
jeeshofone wants to merge 1 commit into
kirodotdev:mainfrom
jeeshofone:feat/fork-merge-back

Conversation

@jeeshofone

@jeeshofone jeeshofone commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

Forking a session is a one-way door. A fork exists to try something without polluting the parent — a side investigation, a risky refactor conversation, an alternative approach — but once the experiment pays off there is no way to bring its outcome home. The user is left with two permanently diverged sessions: the parent never learns what the fork concluded, and the fork lingers in the open-tab list as a stale near-duplicate that has to be closed by hand and re-found in History later.

Why it matters

Fork-heavy workflows (issue #3816) currently punish exactly the users who adopt them: every successful side-quest costs a manual copy-paste of its conclusions back into the parent plus tab housekeeping, and every skipped copy-paste means the parent's later turns proceed without context the user already paid tokens to produce. Merge-back turns the fork lifecycle into a loop — branch out, conclude, fold the conclusion back in — which is what makes forking safe to use routinely.

What changed (motivation → approach → change)

Goal: fold a finished fork back into its parent. The maintainer ruled on the design questions in the issue thread: a visible marked block (not silent context injection), summary only (not raw transcript replay), appended at the parent's current tail with a gap note when the parent advanced, and the fork is archived after merging. This PR implements exactly that shape, reusing existing machinery at every step rather than inventing parallel paths:

Lifecycle, stated plainly (review): merge-back is itself one-way in v1. The merged fork becomes read-only — it cannot be continued, regenerated, edited, or merged again (already_merged), and a fork of a merged fork cannot merge into it either (409 parent_merged, its own terminal code). "Archived" here is stronger than "closed but resumable": the fork stays readable from History and that is all. The confirm dialog says this before the user commits. A repeatable/incremental merge is the proposed v2 follow-up.

  • src/kiro_crew/dashboard/chat_merge_back.py (new) — POST /api/chat/slots/{slot}/merge-back on a fork. Resolves the parent from forked_from (open slot preferred so the block renders live, else rehydrated from History; handles channel-origin keys), summarizes the whole fork transcript with the existing on-demand session summarizer (force=True; the summary is read back from the sidecar rather than trusting the generator's bool), computes the fork point via an identity-based longest-common-prefix scan (per-message mid preferred) to produce the message_range and the "parent advanced N message(s)" gap note, appends a merged_summary block at the parent's tail, and archives the fork.
  • Concurrency + failure semantics (hardened in local review): the whole transition is serialized on the slot's existing _fork_lock; both transcript reads run via asyncio.to_thread (the AUTOSDE no-blocking-call-on-event-loop rule); a stale summary (refused/failed regeneration over newer persisted turns) is rejected with 409 rather than committed; the parent block is broadcast only after its durable save confirms (no phantom rows in open tabs); and an archive-save failure keeps the merge fact (_merged=True + _archive_pending) so a retry re-runs only the archive step — the parent can never gain a duplicate block. A cross-restart belt-and-braces scan of the full on-disk parent transcript covers the marker-loss case.
  • chat_persistence.py / history.py / state.pymerged / merged_into / merged_at become slot-owned metadata; merged folds into closed on every save (including periodic flushes of a History-resumed merged fork, which previously would have dropped the closed marker and revived the archived fork on restart), so restore paths keep a merged fork readable but non-continuable with no new restore-path code.
  • chat_handlers.pyapi_chat and the resume path refuse turns on a merged slot, making the archived fork read-only.
  • FrontendMergedSummaryCard renders the block (GitMerge icon, fork title, localized gap note, full-fork-coverage label) in both transcript surfaces (transcriptRenderers.tsx registry + ChatPage's renderer chain); the fork breadcrumb in ChatPane gains a confirm-gated "Merge back" action (hidden once merged); mergeBackChatSlot API client method + mergeBack mutation in useSessionActions that switches to the parent on success and maps the endpoint's distinct 409 codes to specific messages.
  • i18n — 11 new keys with real translations across all 11 target locales, en-XA regenerated.

Restructure round (head 0cdbe70ad, after three rounds of blocking findings in the same span): enforcement moved from per-endpoint checks to the mutation boundary — _ChatSlot.append refuses live rows on a merged/merging fork (SlotMergedError), _run_chat refuses turns at entry, and a shared merged_slot_response 409 gate now covers send, continue, regenerate, variant switch, edit/resend and rewind; the parent is reserved (_merging) for the whole transition with a fast parent_busy 409 before the summarization spend; merge idempotency is content-keyed (merge_key = hash of the fork snapshot, content_sig tamper check, merge_receipt_corrupt on mismatch) instead of positional; the pane header's split/close icons collapsed into one overflow DropdownMenu so the row stays under the two-control cap with the fork breadcrumb trigger; the breadcrumb pill itself shows "Merging…" while pending. Subtractions per review: persisted gap_note fallback, merged_into snapshot field, retried:archive response key, and the never-written merged_at meta key are gone.

Review-round changes (heads 584654cab8055e7287): the fork is held read-only for the whole merge transition (_merging flag + locked re-check of running/queued work); the parent-block idempotency scan is range-matched so a stale block from a pre-restart merge is never accepted as current; the parent's delivery queue is no longer drained (only the block's own pending entry is removed); the summaries-disabled gate is lifted for this one explicit call so the action works on a default install; the 200 body is trimmed to {ok, parent_key}; archive_failed/parent_save_failed/parent_missing/merge_in_progress map to distinct localized alerts; the merge button shows a pending state; the gap note is built client-side from a structured advanced count (i18next plurals, 12 catalogs).

Mechanical ride-alongs: the locale catalogs re-sort (output of the repo's i18n sort gate) and one routes/chat.py line reformat (formatter gate) — no behavior change in either.

Round 7 (head 3ed7976bf): the parent-close race is fully closed — the shared close_slot teardown now refuses (409 merge_in_progress) on the parent's _merge_reserved reservation as well as the fork's _merging, so a close raced against the merge's durable save can no longer be silently undone; resume sets the merged read-only flag AFTER the history replay loop (flag-first tripped the append write gate and made every merged fork unreadable from History); the merge-back error alert maps fork_flush_failed and non_persistent_session to accurate localized copy instead of the speculative "parent may be gone" fallback; "Open parent" resumes a closed parent through the History resume path (single view AND split-pane collapse) instead of silently 404ing; the module spec (docs/system-specs/modules/learn-cron-dashboard.md) documents /merge-back and the slot-owned merged meta key; and nine unrelated main-side screenshot files the rebase had silently deleted are restored.

Known limitation (v1 scope): the merged summary reaches the parent's next turns via the in-memory pending-context queue; the persisted merged_summary row is presentation-only (it is not in RECALL_ROLES), so after a gateway restart the parent's cold replay does not re-inject the summary into the model — the visible card remains. Making the persisted row recall-eligible touches the recall policy for all roles and is deferred to a follow-up rather than smuggled into this PR.

Tests

  • TestMergeBackSlot (14 tests, test/test_dashboard_chat.py): head-fork happy path (block content, range, archive); tail-fork range; gap note when the parent advanced; parent-not-open rehydration; double-merge 409; running-turn 409; stale-summary 409; summary-unavailable 409; not-a-fork 409; unknown slot / missing parent 404s; non-persistent 400; parent-save-failure 503 with clean rollback; archive-failure retry (503 archive_failed keeps _merged, retry archives without duplicating the parent block).
  • test_merged_flag_folds_into_closed_on_periodic_save locks the persistence fold.
  • Frontend: renderer + card tests via the existing component suites (940 tests across the changed components pass); full i18n suite (616 tests) + i18n:check green.
  • Gates: targeted pytest 59/59 + the persistence/restore classes (101 total), isort/flake8/mypy, tsc -b, vitest.

Manual verification

Local review round (GPT + Opus mirrors of the repo's CI review contracts) found 5 blocking/advisory defects — all fixed and re-verified by a focused verification pass confirming each closed with no new Critical/High. UI screenshots below were captured from a seeded dev instance (same-origin against the built dist); component behavior is additionally locked by tests.

Screenshots / video

Captured from a seeded dev instance (parent "Retry logic investigation" + fork "Alternative cap design").

Parent session — the MergedSummaryCard block appended at the tail (fork title, summary body, italic gap note from the structured advanced count):

Parent session showing the Merged from Backoff deep-dive summary card

Fork pane (Split View) — the breadcrumb pill is the dropdown trigger holding "Merge back" (no extra header-row button; menu open):

Fork pane with the forked-from breadcrumb dropdown open showing the Merge back item

Related Issues

Fixes #3816

Checklist

  • Single commit with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

@jeeshofone
jeeshofone requested a review from a team August 21, 2026 12:20
@jeeshofone
jeeshofone requested a review from a team as a code owner August 21, 2026 12:20
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 21, 2026
@jeeshofone
jeeshofone force-pushed the feat/fork-merge-back branch from 79bb2d5 to 347c713 Compare August 21, 2026 12:53
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 21, 2026
@jeeshofone
jeeshofone force-pushed the feat/fork-merge-back branch from 347c713 to be89cfa Compare August 21, 2026 13:11
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 21, 2026
@jeeshofone
jeeshofone force-pushed the feat/fork-merge-back branch from be89cfa to 1eddcf3 Compare August 21, 2026 13:36
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 21, 2026
@jeeshofone
jeeshofone force-pushed the feat/fork-merge-back branch from 1eddcf3 to ef06640 Compare August 21, 2026 14:12
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 21, 2026
@jeeshofone
jeeshofone force-pushed the feat/fork-merge-back branch from ef06640 to 584654c Compare August 21, 2026 23:35
@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 32 disposition (fixes on 6e8b11703)

F1 (security — app-id equality does not authorize the parent transcript): FIXED as suggested. The round-11 parent check compared _app for equality, but owning the slot does not imply owning the session or transcript the merge writes into — a parent slot named like a channel-session stem with channel_origin set writes into the channel's own transcript while every _app comparison passes. The parent is now routed through the SAME shared gate every slot-addressed write uses (_check_slot_app_ownership: app ownership + linked-session + transcript-key conditions), keeping the 404-not-403 shape and SEL logging. Function-local import with the standard cycle rationale (chat_handlers imports this module). New test constructs exactly the flagged shape — same _app, channel-stem parent, channel_origin set — and asserts 404 with nothing merged and no block on the channel transcript.

F2 (security — fork title can escape the pending-context frame): FIXED, taking the omit option. The title is model/user-influenced text and was interpolated into the frame LABEL — ahead of the untrusted-data preamble that guards only the content. source is now the constant "merged fork session": the visible merged card already names the fork (merged_from_title, which stays redacted as before), and a constant source keeps all merges in one per-source cap bucket rather than letting hostile titles mint unbounded buckets. Chose omission over sanitization because a stripped title in a frame label serves no consumer — the card is the human surface, the content is the model surface, and the label needs neither. Adversarial test drives a title carrying newlines + directive text through a real merge and asserts the source is the constant with no leakage; the round-6 enqueue test now also pins the exact source value.

Verified: merge suite 55/55, black gate, flake8, mypy.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 33 disposition (fix on 57e4621f3) — and the round-31 consolidation pre-commitment delivered

F1 (fenced — crew completions stranded during merge): FIXED as suggested. The crew terminal marks the agent done BEFORE on_subagent_done delivers, so neither running nor the workflow registry shows the delivery window. Two pieces:

  1. CrewOrchestrator now exposes has_pending_work_for(slot_key) reading its two books — owned runs (_owned) plus a new _delivering window marker that on_subagent_done holds from its _owned pop until the durable forward lands (try/finally), closing exactly the gap the finding names.
  2. The merge treats pending crew work as busy: 409 crew_delivery_active, retry after delivery. The probe fails closed — an unanswerable probe is not an answer of quiescent, and this is a data-loss guard.

Consolidation (announced in round 31, triggered by this fourth member): fork_busy_for_merge(state, slot) is now THE single busy predicate — running turn / autopilot stage, active workflow run, pending crew work — consumed at BOTH seams (entry fast-fail and the round-30 under-lock re-check). A future member is added in one place and automatically checked at both; the crew/workflow probes are synchronous reads, so the under-lock check stays loop-atomic with the reservation. The busy family now mirrors the driver-side story: merge_transition_active is the one predicate for "the merge holds this slot", fork_busy_for_merge the one predicate for "this slot holds work the merge must wait for".

Tests: crew-pending → 409 → quiescent → same POST succeeds end-to-end; a direct orchestrator test pins both books of has_pending_work_for (owned-run and delivery-window). Merge suite 57/57, full crew suite 164/164 (the on_subagent_done split is exercised by every existing completion test), black gate, flake8, mypy.

CI note: the Backend Lint & Type Check failure on the previous head does not reproduce locally — black gate, flake8 (full src), isort, and mypy over all 1303 source files all pass here; this push re-runs the job, and I will read its log if it stays red on an otherwise-green head.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 34 disposition (fixes on 1d4039774) — GPT F1, Opus F1, and the recurring lint red all resolved

GPT F1 (fenced — merge discards accepted pending context): FIXED as suggested, expiry-aware. Live _pending_context entries now join fork_busy_for_merge as its fifth member: an accepted /context or /note entry drains into the fork's NEXT turn, which a merge ensures never comes — archiving input whose producer was told 200 and which the frozen summary never saw. 409 pending_context_waiting, with copy telling the user the two exits (run a turn to consume it, or let it expire). EXPIRED entries deliberately do NOT block — filtered with the same context_entry_expired predicate the drain uses — so a fork whose only entries have aged out merges normally instead of deadlocking on context nothing will ever deliver. Test drives both halves: live entry → 409; expired-only → the same POST succeeds.

Opus F1 (blocking — _delivering set cannot represent concurrent same-slot deliveries): FIXED as suggested. The round-33 marker is now a reference COUNTER (dict[str, int]): each concurrent completion delivery holds one count, decremented in its own finally, so the first to finish can no longer clear a sibling's window. The probe (slot_key in self._delivering) is unchanged. The orchestrator test now pins the counter semantics: at count 2, releasing one leaves the slot pending.

Backend Lint & Type Check (red on the last two heads): root-caused and fixed. CI's flake8 covers test/ + conftest.py, which my local floor did not — a round-32 bulk edit added slot_history_key to four function-local imports where only one function uses it, producing four F401s invisible to my src/-only flake8. The three collateral imports are reverted (the used one stays); my pre-push floor now runs CI's exact flake8 scope. Verified: flake8 src/kiro_crew test conftest.py xdist_budget.py clean.

Merge suite 58/58, crew suite 164/164, black gate (crew_chat.py deliberately kept on the known-unformatted baseline — the counter edit is formatted in the file's existing style rather than graduating 170 unrelated lines into this PR), mypy clean.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 35 disposition (fixes on 38e6f051c)

F1 (fenced — reserved parent permits doomed auto-turn prompts): FIXED as suggested, both halves. A workflow completion's result row legitimately lands on a _merge_reserved parent (the round-6 one-shot-delivery carve-out), but its auto-turn synthesis prompt took enqueue_or_run_prompt's RUN branch — appending a user-role row and dispatching straight into the turn gate's refusal, leaving the prompt persisted with no answer ever coming. Two changes:

  1. Park side: enqueue_or_run_prompt now treats merge_transition_active(slot) as busy — the prompt is QUEUED (append deferred to the drain, which re-runs every admission gate at delivery). The predicate moved from chat_utils into state.py (chat_utils re-exports, importers unaffected) because _ChatSlot itself now consults it and the import direction only works that way.
  2. Release side: the reservation finally now kicks the parent's queue (_start_next_queued_turn, fire-and-forget, best-effort) — nothing else drains a queue on an IDLE slot, so without the kick a mid-merge prompt would wait for the next unrelated turn.

Tests: a reserved slot queues the prompt (returns False, no user row, dispatch coroutine asserted never called); an end-to-end merge whose summarizer injects a queued prompt mid-reservation asserts the kick fires for the parent after a 200. The first version of the kick test parked the prompt BEFORE the merge and correctly got 409 parent_busy — kept as designed, the test now injects mid-merge, which is the only window the kick exists for.

F2 (fenced — paused workflow treated as finished): FIXED as suggested. _WORKFLOW_STATUS_RUNNING became _WORKFLOW_ACTIVE_STATUSES = {"running", "paused"} — a paused run resumes and delivers, so only terminal statuses are quiescent. The constant-pin test now asserts set equality against STATUS_RUNNING/STATUS_PAUSED, and a new test drives a paused run through the endpoint (409 workflow_run_active).

Backend Tests (3.12, 4) red on the previous head: the failure is test_snapshot.py::TestNotificationCopyWhenNoLiveFileExists... — a snapshot/notification-ordering test in a subsystem this branch has never touched; treating as main-inherited/flaky pending this head's run, and I will verify on a detached main checkout if it recurs.

Merge suite 62/62, guardrail suites green, black gate, flake8 (CI scope incl. test/), mypy clean.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 36 disposition (fixes on a4941a319)

F1 (fenced — merging forks accept prompts that archival discards): FIXED as suggested, and it is a correction to my round-35 breadth. Round 35's queue condition used merge_transition_active, which includes _merging — the FORK side — so a heartbeat or session-send landing on a mid-merge fork was parked in a queue its archive then discarded: silent loss, strictly worse than refusal. The condition is now exactly what the finding prescribes: queue on running or _merge_reserved (the parent, whose release kicks the drain); merged/merging slots fall through to the run branch, whose append raises SlotMergedError loudly for the caller's failure handling to own. On the record: round 35 traded a loud failure for a silent one on the fork branch — the corrected split (parent defers, fork refuses) is the asymmetry the two flags have always encoded, and I should have carried it into the queue condition.

F2 (fenced — parent reservation strands subagent completions): FIXED as suggested. The subagent completion dispatcher's _injection_slot_busy probe read only running/task, so a completion arriving during the parent's reservation took the idle branch and dispatched _run_chat directly — whose turn gate silently returns on _merge_reserved: neither ran nor queued. The probe now counts _merge_reserved as busy, routing delivery down the existing wait-then-queue path; the round-35 release kick drains it. Deliberately not _merged/_merging in this probe — a merged fork's own completions are the write gate's business (F1's loud path).

Tests: the round-35 unit test now also drives the round-36 fork branch (merging fork → SlotMergedError, queue stays empty); a new probe test pins _injection_slot_busy on the reservation flag. Merge suite 62/62, heartbeat + gateway-coverage suites 146/146, black gate, flake8 (CI scope), mypy clean.

The Backend Lint red on the previous head remains annotation-opaque (".github:16 exit 1") and does not reproduce locally against CI's exact flake8/black/mypy scope — I'll pull the completed run's log next cycle and root-cause it there if this head's run stays red.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Status + CI fixes on 97d2fe714

All three AI lanes are GREEN on the round-36 head (a4941a319) — GPT, UX, and Opus all pass. The remaining reds were mechanical, all root-caused and fixed:

  • Backend Lint (red for three heads): root-caused at last — the completed run's log names it: isort on chat_merge_back.py. The round-35 _start_next_queued_turn import was inserted by a targeted edit out of sorted position, and my local floor's isort check didn't cover the window between edits. File isort-fixed; local floor now runs isort --check-only src/ test/ before every push.
  • Backend shard 4 + Windows (2)/(4): the rounds-20–24 test-double class, fourth occurrence — round 36's _injection_slot_busy reads _merge_reserved, and MagicMock() slot doubles auto-mint a TRUTHY attribute for it, so every injection in test_subagent_scale.py (11 failures), test_cron_thread_routing.py (3), and test_subagent_delivery_ttl_anchor.py took the busy path and delivered nothing. Modeled _merge_reserved = False on all 21 doubles across the three files (keyed on the probe's callers, per the established sweep rule). Suites now 57/57, 183/183 locally.

No production code changed beyond the isort reorder. If this head comes back clean, #4904 is fully green on rounds 26–36 inclusive.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Status note — GPT formally CLEAR; rate-limited run retriggered

The GPT lane's comment on the round-36 head (a4941a319) reads "✅ no blocking findings" — after 36 review rounds the security lane is formally clear. The failures shown on 97d2fe714 (both AI lanes plus five infrastructure jobs: Detect changed surface, Coverage Gate, Screenshot Evidence, Resolve desktop matrix, Publish readiness signal) are all the same cause, confirmed via job annotations: GitHub API rate limiting on the runner installation ("API rate limit exceeded for installation"), not code. 055117913 (content-identical, new SHA) is pushed to retrigger the full suite once the installation's quota window resets.

No production or test changes on this push.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 37 disposition (fixes on ace98238b) — the finding, its twin, and a drain-side hole the audit found

F1 (fenced — cron delivery to reserved parent silently discarded): FIXED as suggested. The cron-delivery idle branch in handlers/messaging.py gated only on running/_in_stage_execution, so an idle reserved parent took the direct-dispatch branch into the turn gate's silent return — delivery reported success, turn never ran. _merge_reserved now routes to the queue branch; the round-35 release kick drains it with every admission gate re-run.

Proactive audit — rounds 21/35/36/37 are one recurring shape (a direct dispatcher missing the reservation), so instead of waiting for round 38 I enumerated every spawn_guarded_turn / direct _run_chat dispatch site and audited each. Two more changes came out:

  1. slack/gateway.py's cron-delivery twin (~line 3380) had the identical gap — and gated on running alone, missing _in_stage_execution too. Brought to full parity with the messaging.py site (both flags + the reservation).
  2. The queue drain itself (_start_next_queued_turn) could fire from an unrelated seam (another turn's chat_done, a note flush) DURING the reservation and pop a parked entry straight into the silent gate — the entry consumed, the turn never run, which would have quietly defeated every park-in-queue fix from rounds 35–37. The drain now holds while _merge_reserved and returns without consuming; the release kick re-enters after the flag clears.

The remaining audited sites are clean by construction: chat_runner's two dispatches run FROM the drain (now held); chat_handlers' site is the user-send path behind merged_slot_response; gateway:6297 mirrors the send path; taskrunner's two sites target its own runner slots, which are never fork parents (and its summary turns would surface loudly if that ever changed).

Tests: drain-hold test (reserved → False, entry NOT consumed); merge suite 63/63; cron routing 38/38; queue-family suite 34/34. Black gate, flake8 (CI scope), isort (src+test), mypy clean.

Backend shard 2's test_job_sdk.py liveness failure is in a subsystem this branch has never touched — watching for recurrence before spending a detached-main probe on it.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 38 disposition (fixes on 8c34d03af)

F1 (fenced — parent rebind bypasses app isolation): FIXED as suggested, both halves. The ownership gate authorized one observation of the parent's routing, then the 10–30s summarization await left a window where cron/workflow delivery could rebind linked_session_key ungated — landing the merged block on a transcript the caller never authorized. Now:

  1. The authorized key is captured AT the ownership gate (slot_history_key(parent_slot), single observation) and threaded into the transition body.
  2. Re-authorization after the summarization await: a routing move is refused with a clean 409 parent_rebound (SEL-logged, source="routing_moved") BEFORE the block is appended — nothing to roll back, fork untouched, retry-safe. Applies to every caller, app or not: the rebind race exists independent of app isolation.
  3. The save is pinned with expected_history_key=<authorized key> — the persistence layer's snapshot-then-confirm guard refuses the write (returns False, nothing written) if routing moved between the recheck and the flush snapshot; the existing round-8 rollback path handles the False.

The recheck catches the 10–30s window cheaply; the pin closes the residual gap between recheck and flush atomically inside the guard the persistence layer built for exactly this race.

Test: wrapping summarizer rebinds the parent mid-merge → 409 parent_rebound, parent message count unchanged, fork not merged. Merge suite 64/64.

CI shard-4 red on the previous head was the doubles class again (test_slack_gateway_cron_exec_coverage's _mock_slot MagicMock auto-minting truthy _merge_reserved/_in_stage_execution against round 37's gateway guard) — both flags now modeled explicitly; that suite + appkit endpoints 213/213. Black gate, flake8 (CI scope), isort, mypy clean.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

CI note on 8c34d03af: all backend/frontend test shards are green; the failing wall (Detect changed surface → Coverage Gate failing closed, review lanes, screenshot evidence) is GitHub API rate limiting on the runner installation again ("API rate limit exceeded for installation" in the job annotations — same signature as two runs ago). Content is unchanged from the round-38 fix; I'll retrigger once the installation quota has had time to recover rather than burning the window now.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Re-sync #15 + shard-2 fix (6b84f3691). Main moved under the branch (#7573 app session controls) and turned the PR CONFLICTING: two additive hunks in ChatPage.tsx (imports + the component-body block where main's session-control state and our merged-fork state anchor at the same point) — resolved as a union, main's side first. tsc -b, merged-fork + ChatPage vitest (21/21), and all i18n gates green after resolution.

The shard-2 red on the previous head was the MagicMock-doubles class in a THIRD file: test_dashboard_file_io.py's three mock_slot doubles modeled running/_in_stage_execution but auto-minted a truthy _merge_reserved against the round-37 delivery guard, diverting the origin-inject tests onto the queue branch (where queue_append's Mock return broke json.dumps). All three now model the flag; suite 44/44 (which also picks up main's a7bfe44c7 ACL-test fix via the rebase).

Merge suite 64/64; black/flake8 (CI scope)/isort/mypy clean.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 39 disposition (fixes on f4c04850f)

F1 (fenced — bulk cleanup can remove a reserved parent mid-merge): FIXED as suggested, both halves. api_chat_slots_cleanup's 3-day idle heuristic is exactly wrong for a reserved parent: an old parent is idle by nature (its last visible activity can be weeks old), so it reliably lands in stale_keys while a fork's merge is actively holding it — and the pop would let the merge rewrite it open-but-absent. Two guards, per the suggested shape:

  1. Selection skip: the per-slot loop now exempts merge_transition_active(slot) — the same consolidated predicate every other driver uses (close entry, deletion guard, nudge dispatcher, gateway send).
  2. Pop re-check: selection runs before the archive loop's awaits, so a merge can reserve a selected parent in between. The pop now re-reads the slot and skips if the transition became active — closing the TOCTOU window the same way the round-30/25 lock-atomic re-checks do on the reservation side.

Sibling audit (the round-37 lesson applied proactively): every other state._slots.pop site was checked — openai_compat's three (freshly_created/ephemeral only), chat_persistence's rollback (this-call-created only), chat_fork's two (the fork this call minted) — all pop only slots they themselves created, so none can reach a reserved parent. The single-tab close and delete paths already adopted the predicate in round 26.

Test: reserved parent + merging fork + plain stale slot, all 10 days idle → cleanup archives ONLY the plain one, both transition slots survive with their flags intact. Cleanup + merge + queue-drain suites 76/76. Black gate (one wrap in the new test, applied), flake8 (CI scope), isort, mypy clean.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Coverage Gate fix (b6e863b74). The round-39 head cleared every lane and every test shard — GPT posted ✅ no blocking findings — with a single remaining red: the frontend per-file floor caught SessionActionsMenu.tsx at 76% (the MergeBackMenuItem component and its render site were the uncovered regions; the file's line count shifted under re-sync #15, tipping it below 80%). Added four tests covering the item's full behavior: renders for a fork and merges after confirm (prompt names the parent TITLE), declined confirm does not merge (prompt falls back to the parent key), pending state disables the item and blocks the action, and merged/plain slots render no item. File now at 96.15% statements. tsc clean; content otherwise identical to the lanes-green head.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 40 disposition (fixes on 36a40df95)

F1 (fenced — rollback can discard a concurrent parent delivery): FIXED as suggested. The tension is between two deliberate designs: the round-25 rollback unpublishes a parent this call rehydrated when the merge fails pre-commit, and the round-6 reservation deliberately keeps accepting one-shot background delivery appends on that parent (only its TURNS are blocked). A heartbeat/cron row landing during the summarization await therefore lives only in memory on the freshly rehydrated slot — and the unconditional pop discarded it.

The pop is now gated on an unchanged rehydrated baseline: total_messages (the lifetime append counter, survives trimming) is captured at the moment this call mints the slot, and the rollback pops only when it has not moved. A moved counter means the slot carries new content — it is RETAINED (the ordinary flush paths persist a live slot, and the reopened tab is now justified by the delivery it holds), exactly the "retain or persist" shape the finding suggested. The identity check (is parent_slot) stays as the first condition.

Tests: (1) delivery lands mid-summarization + summarizer refuses → 409, parent survives carrying the delivered row, fork untouched/retry-safe; (2) counterpart — no delivery, pre-commit failure → parent still rolled back exactly as round 25 specified. Merge suite 67/67; black/flake8 (CI scope)/isort/mypy clean.

For the record: the rollback family is now triple-guarded — creation witness (r26: only pop what this call minted), commit witness (r27: never pop post-commit), unchanged baseline (r40: never pop new content).

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Re-sync #16 (229bdf3e5). Main moved again (five merges including the #9089 security path-fence refactor) and turned the PR CONFLICTING before the round-40 head's CI could run. One conflict hunk: ChatPage.tsx's chatSlice import list — main cleaned up imports its page no longer uses. Resolved by taking main's list and re-adding only the two symbols this branch genuinely uses (resumeFromHistory for the merged bar's Open-parent resume path, clearMergeBackError); fetchHistory had no remaining usage on either side, so it stays dropped.

Verified: tsc, 84 merged-fork/menu frontend tests, all 10 i18n gates, backend merge suite 67/67 (including the round-40 rollback-baseline pair), black/flake8 (CI scope)/isort clean. The round-40 content is unchanged — this head is that fix rebased.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Re-sync #17 (9ae59136e, conflict-free). The Windows shard-3 red on the previous head was inherited: main's #9089 security path-fence refactor removed a helper that test_security.py's chained-cd gate test still patched, and main itself has since shipped the repair (#9182, cbdd4a569). This branch never touches security.py or its tests — rebasing onto current main picks up the repair. Verified locally: the exact failing test now passes, merge suite 67/67, black gate clean. Round-40 content unchanged.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 41 disposition (fixes on 05e8ac09c)

F1 (fenced — parent reservation can attach to a closed, stale slot): FIXED as suggested. Parent resolution runs before the lock acquire, so a close (or variant switch) can pop the parent from the registry in that window; the round-25 lock-atomic reservation then landed on the popped OBJECT, and the merge's durable save would write the closed session back open. The reservation block now verifies state._slots.get(parent_slot.key) is parent_slot as its FIRST condition under the lock — the registry read is loop-synchronous, so the identity check and the reservation set are one atomic step. On mismatch it aborts retryably (409 parent_rebound; a retry re-runs authorization against the new state) rather than re-resolving in place, which would skip the ownership gate the earlier resolution passed through. A freshly rehydrated parent passes unchanged: rehydration publishes into the registry before this point.

Chose abort-retryably over the suggestion's re-resolve alternative deliberately: re-resolving under the lock would attach the merge to a slot object the round-13/32 authorization gates never examined.

Test: hold the parent's lock, start the merge (blocks at acquire), pop the parent and publish a replacement under the same key, release → 409 parent_rebound, neither the stale object nor the replacement reserved, fork untouched. Merge suite 68/68; black gate, flake8 (CI scope), isort, mypy clean.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 42 disposition + re-sync #18 (fixes on 22aea4254)

F1 (fenced — script cron result lost during fork merge): FIXED as suggested at BOTH delivery sites. Round 37 taught the cron dispatchers about the reserved PARENT; this round is the FORK side of the same delivery: a merging fork's transcript is frozen by the round-28 write gate, so a direct append raises SlotMergedError and the script cron's result died in a logged exception (a merged fork can never take delivery again at all). Both the gateway script-result site and messaging.py's origin-delivery twin now null the session target when the fork is _merging or _merged and fall through to their own existing fallback — the channel notification (messaging.py's carries the "session closed — delivered as notification" marker), exactly the deferral-to-notification shape the finding suggested. The result is delivered, not parked: a merging fork's queue would be archived with it (round 36's lesson), so the queue is deliberately NOT the parking spot here.

Doubles: the two new flag reads were modeled (_merging/_merged = False) on every MagicMock slot double reaching the guards — caught locally this time, before CI (5 doubles across test_dashboard_file_io.py + test_slack_gateway_cron_exec_coverage.py). New test: origin delivery to a merging fork appends nothing and notify carries the result. Suites: file_io + gateway cron 120/120, merge 68/68.

Re-sync #18 folded into this push: main moved again; single conflict was error-code-baseline.json, resolved by taking main's and regenerating with the repo tool (compliant: 1745). Black gate, flake8 (CI scope), isort, mypy clean.

(The adjudication section of the review comment shows an infra error this round — "Claude Code 2.1.240 does not support this model" — so the verdict stood unadjudicated; noting for the maintainer, fixed on merit regardless.)

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 43 disposition (fixes on 966d0756d)

F1 (fenced — merge can discard accepted parent context): FIXED as suggested. The round-6 model-visibility enqueue pushes the merge summary through the parent's pending-context buffer, which FIFO-evicts at _MAX_PENDING_CONTEXT (50) — so on a parent already holding 50 live entries, the merge's own enqueue silently discarded the oldest ACCEPTED entry (a delivered cron/subagent result that had not yet reached a turn). Exactly the class rounds 35–42 have been closing from the delivery side, now closed from the merge's side.

The preflight sits immediately before the commit (the block append + durable save), per the suggested shape: expiry-aware live count (context_entry_expired, the same predicate the buffer itself uses) >= _MAX_PENDING_CONTEXT → 409 parent_context_full with an actionable message (run a turn in the parent to consume the context, then retry). Placed at the pre-commit point rather than merge entry so deliveries landing DURING the summarization await are counted — a clean refusal here has nothing to roll back, and the fork is untouched and retry-safe.

Test: parent filled with 50 live entries → 409 parent_context_full, no block appended, zero eviction (oldest entry verified surviving), fork unmerged. Merge suite 69/69; black gate, flake8 (CI scope), isort, mypy clean.

(Adjudication lane errored again this round — same "Claude Code 2.1.240" infra failure as round 42 — so the verdict stood unadjudicated; fixed on merit.)

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 44 disposition (fixes on 2a2dbc862)

F1 (fenced — reserved parents still accept context writes): FIXED as suggested. Round 43's capacity preflight counts live entries immediately before the merge's commit, but POST /context remained writable on a RESERVED parent — so the buffer could fill during the parent save that follows the preflight, and the merge's own enqueue then evicted previously accepted context (the exact loss round 43 closed, re-opened through a different door). The context-append gate now refuses _merge_reserved slots retryably — 409 merge_in_progress, matching the code the turn endpoints already answer during a reservation — right below the round-24 merged/merging refusal. The reservation spans one merge transition, so a retry lands after release; refusal is the right shape here rather than queueing, because pending context has no release-kick drain the way prompts do (round 35), and the producer's retry loop is the existing contract for 409s on this endpoint.

The delivery/merge context family is now closed at every seam: producers gate on merged/merging (r24) and reserved (r44), the merge preflights capacity pre-commit (r43), delivery dispatchers park or fall back (r35–r42), and the drain holds during reservations (r37).

Windows shard-2 red on the previous head was the doubles class again: test_cron_origin_injection.py's two slot factories reached the round-42 _merging/_merged reads — flags now modeled (file #10 in the doubles ledger); suite 6/6.

Tests: reserved parent POST /context → 409 merge_in_progress, buffer untouched; merged-fork r24 test unchanged. Merge suite 70/70; black gate, flake8 (CI scope), isort, mypy clean. (Adjudication lane errored a third consecutive round — same Claude Code version infra failure.)

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 45 disposition (fixes on 9dacb4058)

F1 (fenced — reservation check races the offloaded deletion): FIXED as suggested, full claim protocol. The deletion paths' round-5/22 reserved-keys checks run on the event loop, but the unlink itself is offloaded to a thread — a merge could reserve-and-save in that gap, and the deletion then unlinked the transcript the merge had just durably written (fork archives, summary lost). The suggested three-part protocol is implemented exactly:

  1. Deletions claim synchronously before offloading: a new state.deletion_claimed_keys set (symmetric counterpart to merge_reserved_keys, same immutable-baseline pattern) — both the single-delete endpoint and the bulk clear add {key} ∪ transcript_stems(key) on the loop, atomic with their reserved-check, before asyncio.to_thread(delete_session, ...).
  2. Merges reject claimed keys: the coordinator checks deletion_claimed_keys ∩ _reserved before registering its reservation → 409 deletion_in_progress (retryable — a claim spans one unlink), unwinding the parent's _merge_reserved flag on the refusal path (no awaits between set and check, so nothing can have queued against it).
  3. Claims released in finally on both deletion paths.

Both spellings are claimed for the same reason the merge reserves both (round 6): the History surface and slot_history_key address the same transcript differently, and transcript_stems covers legacy stems (round 22).

Test: a claim on the parent's history key → merge answers 409 deletion_in_progress, nothing reserved, flag unwound, fork untouched. Suites: merge+deletion 72/72, sessions memory+clear 26/26 (they exercise both real deletion paths through the claim/release cycle). mypy (3 files), black gate, flake8 (CI scope), isort clean.

(Adjudication infra note: #9216 has been filed and claimed upstream for the "Claude Code 2.1.240" adjudicator failure — rounds 42–45 stood unadjudicated.)

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 46 disposition (fixes on 79d35e9b9, includes re-sync #19)

F1 (fenced — failed nested merge resurrects an archived parent): FIXED as suggested. The round-25 rollback wrapped only the transition body's response, so a refusal issued AFTER the rehydrate but BEFORE the transition — the busy/merged 409 on an archived merged parent (the named case), the round-32 app-ownership 404, and the round-45 deletion-claims 409 — left the rehydrated session published: the failed merge's one visible effect was reopening a session the user had archived.

The rollback is now a shared helper (_rollback_unconsumed_rehydration) applied at every post-rehydration failure exit, preserving both existing guards: the round-26 identity check (only the slot THIS call minted) and the round-40 unchanged-baseline check (total_messages — a delivery that landed in between justifies the reopened tab and is retained). The round-41 parent_rebound exit is deliberately not wrapped: it fires exactly when the registry already holds a different object, so the identity guard would no-op — the stale object is already unpublished.

Test: archived MERGED parent + child merge → 409 parent_busy AND the parent is NOT left published. Merge-family suite 75/75; black gate, flake8 (CI scope), isort, mypy clean.

Re-sync #19 folded in (clean, no conflicts). Note on the previous head's Windows shard-1 red: test_acp_frame_replay — this branch is byte-identical to main on the ACP code, the harness, and the fixtures, and the same 6 snapshot tests fail at the rebased base locally, so it is main's own red (the #9177 harness refactor's fixtures lag it), not introduced here.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 47 disposition (fixes on 288ef8e09)

F1 (fenced — overlapping deletes can release another request's claim): FIXED as suggested. The round-45 claims were a plain set, so two concurrent deletes of the same key each |='d and each -='d — the first delete's finally cleared the claim while the second's offloaded unlink was still in flight, reopening exactly the round-45 window (merge reserves and saves, second unlink loses the summary). This is the same set-vs-refcount defect class this PR already fixed once on the crew _delivering marker in round 34; acknowledged that the pattern should have been applied here from the start.

deletion_claimed_keys is now a reference-counted mapping (dict[str, int]) managed by two helpers: _claim_deletion_keys increments per key (always writing to an instance-owned dict, never the class-level baseline — the shared-mutable-default trap the old swap-in pattern also guarded), _release_deletion_keys decrements and drops keys at zero, with an idempotent floor so an exception-path double-release cannot go negative. Both delete paths claim/release through the helpers; the merge coordinator's refusal is now key-membership against the mapping (present iff some deletion still holds it).

Tests: the new refcount test walks the exact failing interleaving — two overlapping claims on one key, first release leaves the key CLAIMED, last release clears it, over-release is a no-op; the round-45 merge-refusal test updated to the mapping shape. Merge/deletion suite 76/76, sessions memory+clear 26/26; black gate, flake8 (CI scope), isort, mypy (3 files) clean.

(Windows shard-4 red on the previous head was test_work_ledger's two-conductors race test — this branch does not touch the work ledger; watching for recurrence before treating it as anything but main-side flake.)

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 48 disposition (fixes on 5b0820e5a, two findings)

F1 (fenced — overlapping merges can prematurely release a shared reservation): FIXED as suggested (refcount option). merge_reserved_keys had the same plain-set flaw round 47 fixed on the deletion claims — two transitions sharing a key (reachable through the legacy transcript-stem fallback) each |='d and each -='d, so the first completion cleared the second's still-live reservation and a deletion could then remove the second merge's committed summary. Now the identical refcount mapping and helper pair (_reserve_merge_keys/_release_merge_keys, instance-owned dict, idempotent zero-floor release). Every consumer (both deletion paths, the creation guard in state.py) checks membership, which is unchanged on a mapping. Chose refcounting over the reject-overlap alternative for symmetry with round 47 and because a stem collision between unrelated sessions should not make one of them unmergeable.

F2 (fenced — receipt reuse accepts corrupted summary content): FIXED as suggested. The merge_key signs the SOURCE (fork + snapshot) but nothing bound the receipt to the block's own BODY — a block whose content was corrupted or rewritten after commit still matched, and the retry skipped summarization and archived the fork behind bad parent content. The block meta now carries content_sig (domain-separated SHA-256 of the rendered summary, minted at commit), and the reuse scan verifies the block's CURRENT body hashes to it. A mismatch — or a pre-round-48 block without the field — is not a receipt: the retry falls through to a fresh summarization, consistent with how the scan already treats legacy blocks without a merge_key.

Tests: (F1) the exact interleaving — shared key, two reservations, first release leaves it protected, over-release no-op; (F2) commit + archive-fail, tamper the block body, retry → fresh summarization runs (counted) instead of reuse; the intact-receipt reuse test still passes unchanged. Merge/deletion/receipt suite 82/82, sessions 26/26; black gate, flake8 (CI scope), isort, mypy (3 files) clean.

@iamwhatever iamwhatever left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge-back is terminal, and the fork loop dead-ends

The design ruling in #3816 (archive the fork after merging) makes merge-back a one-way door, and I think the resulting lifecycle is narrower than the "branch out, conclude, fold back in" loop the description promises. Three concrete points:

1. A merged fork cannot be continued at all. After the merge, merged_slot_response refuses send / continue / regenerate / variant switch / edit-resend / rewind with 409 session_merged, and _ChatSlot.append refuses live rows outright. So the natural follow-up question — "I merged, then kept working in the fork, how does the new part get folded in?" — has no answer: there is no new part, because the fork is frozen. Worth stating explicitly in the description, since "archived" reads as "closed but resumable" and this is stronger than that.

2. Re-merging is impossible, and forking around it dead-ends with a misleading error. _merge_back_locked returns 409 already_merged for any merged fork outside the _archive_pending retry window, so a second merge is out. The obvious workaround — fork the archived fork and keep exploring — produces a slot whose forked_from points at the merged fork, and merging that back hits the parent guard in api_chat_slot_merge_back:

if (
    getattr(parent_slot, "running", False)
    or ...
    or getattr(parent_slot, "_merged", False)
):
    return web.json_response(
        {"error": "the parent session has a turn in progress; retry when it finishes",
         "code": "parent_busy"},
        status=409,
    )

A permanently merged parent is bucketed with transient busy states and the user is told to "retry when it finishes" — it never finishes. Please split this into its own code (e.g. parent_merged) with copy that says the parent is archived and cannot receive another merge. As it stands, the fork chain cannot repeat and the failure mode lies about why.

3. The one shot can silently lose its value on restart. The known limitation already noted in the description — merged_summary is not in RECALL_ROLES, so the parent's cold replay after a gateway restart does not re-inject the summary — compounds with (1) and (2): the user gets exactly one merge, and after a restart that merge is presentation-only. One-shot plus non-durable-to-the-model is a rough combination for a feature whose whole point is not losing fork context.

Suggested direction

Preferred: don't archive on merge. Keep the fork writable, and make merge incremental — merge_key is already content-keyed and _post_fork_range already computes a range, so a second merge can diff from the last merge point and append a second card. That is the loop the description describes, and most of the plumbing is here already.

If v1 must stay terminal, then at minimum:

  • give merged-parent its own 409 code and accurate copy (point 2 — this one is a bug, not a scope call);
  • say "this fork becomes read-only and cannot be continued or merged again" in the confirm dialog, not just in the PR body.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Re-sync #20 — main moved (~40 commits, to 2847b5b2d) and made the PR CONFLICTING; rebased. Sole conflict was error-code-baseline.json: took main's and regenerated (contract suite 6/6). No source conflicts. Merge-family suite 82/82 (incl. the round-48 refcount + content-sig tests); black gate, flake8 (CI scope), isort, mypy clean.

For the record: the previous head 5b0820e5a (round-48 content) finished fully green — 68/68 checks, zero failures, GPT ✅ no blocking findings. This push is that same content rebased; the delta is baseline regeneration only.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Thanks — taking this in three parts (fixes pushed in 0fbda471c), with the design question left for the maintainer since it would reverse the #3816 ruling.

Point 2 (parent_busy lies about a merged parent) — fixed as requested. The _merged check is split out of the transient-busy bucket into its own refusal, checked first: 409 parent_merged, copy "the parent session was itself merged and archived; it cannot receive another merge". Frontend maps the new code to matching copy in all 12 locales (en + 11 translations, en-XA regenerated). It keeps the round-46 rollback (the failed nested merge still cannot resurrect the archived parent — that test now asserts the new code), and the docstring's 409 catalogue documents parent_merged as terminal/never-retryable next to parent_busy. Two existing tests updated (merged-parent contexts), the running-parent test still asserts parent_busy.

Point 1 (frozen is stronger than "archived" reads) — dialog copy strengthened. The confirm now says: "This is one-way: the fork becomes read-only and cannot be continued or merged again." (It previously said only "will become read-only".) I'll mirror the same sentence into the PR description's lifecycle section.

Point 3 + suggested direction (incremental merge instead of archive-on-merge) — agreed this is the better loop, but it reverses the #3816 design ruling (archive the fork after merging), which this PR implemented deliberately. You're right that the plumbing is close: merge_key is content-keyed per snapshot and _post_fork_range already computes ranges, so repeat merges could diff from the last merge point and append a second card — and that would also dissolve point 3's one-shot-times-non-durable compounding (the RECALL_ROLES limitation stops being load-bearing when merge is repeatable). I'd rather not flip a ruled design inside round 49 of this PR: proposing it as the follow-up (v2: incremental merge-back — drop archive-on-merge, superseding the tail-fork advisory follow-up which it subsumes). If you or another maintainer would rather have it in THIS PR, say so and I'll implement it here instead.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Round 49 disposition (fix on 8962ef2af)

F1 (fenced — reserved-parent prompts are silently discarded): TAKEN, via the "reject visibly" arm of the suggested fix. The turn gate is the funnel every dispatch shape passes through, and a prompt that reaches it was already ACCEPTED — its user row appended by the caller — so the silent return stranded linked-Slack and OpenAI-compat surfaces exactly as described: no reply, no completion. The gate now refuses visibly, mirroring the refused-remote-slot shape directly above it: on a reserved parent (which is writable) it appends a retryable error row ("a fork is being merged into this session right now … send it again in a moment") and broadcasts chat_done; on a merged/merging fork it broadcasts chat_done alone, because the write gate drops live rows there by design and every fork endpoint already answers 409 from its own pre-check.

Why not the "queue" arm: the queue drain re-appends its message when it dispatches (that is how parked deliveries work), so re-queueing a prompt whose row is already in the transcript would double the user row; and the reservation window is the summarization pass (seconds), so a visible retryable refusal is the honest shape rather than a hidden delay. The r37 park-in-queue behavior for dispatcher-routed deliveries is unchanged — this round only covers the direct-funnel paths that bypass those dispatchers.

Also this push — shard 2 red (both OSes) taken with the same seam: test_issue_radar_crew_mcp_tools._fire_env builds its crew slot as a bare MagicMock, so all three merge flags read truthy and the nudge fire parked. Modeled _merged/_merging/_merge_reserved = False on the double (11th instance of this class; the four sibling nudge harnesses were swept and already pass). Windows shard 4 is the #9317 base-red (cross-PR semantic conflict on main, fix upstream in #9319/#9322) — not addressed here.

Tests: new test_turn_gate_refuses_reserved_parent_visibly (error row present exactly once + chat_done broadcast); merge/drain family 79/79; the repaired harness file 85/85; sibling nudge suites 130/130. black gate, flake8 (CI scope), isort, mypy clean.

@jeeshofone

Copy link
Copy Markdown
Contributor Author

Re-sync #21 (a6c8e00a0): main moved; sole conflict was error-code-baseline.json (took main's, regenerated — contract suite 6/6). Same round-49 content that ran fully green (69/69) before the conflict. Known-red on this push: the new comment-history lane will flag this PR's accumulated round-numbered comments — a dedicated present-tense rewording pass lands next push; it is comment-only and touches no behavior.

POST /api/chat/slots/{slot}/merge-back on a fork resolves the parent from
forked_from, summarizes the fork with the existing session summarizer, and
appends a visible merged_summary block to the parent's tail (with a gap
note when the parent advanced). The fork is archived: merged/merged_into
metadata implies closed, restore paths keep it readable but non-continuable,
and api_chat rejects turns on a merged slot. UI: MergedSummaryCard renderer,
a merge-back action on the fork breadcrumb, i18n in all locales.

Hardened per local review (GPT + Opus mirrors):
- both transcript reads run via asyncio.to_thread (no event-loop stall)
- the transition is serialized on the slot's _fork_lock; a stale summary
  (refused/failed regeneration) is rejected rather than committed
- archive failure keeps _merged and marks _archive_pending so a retry
  re-runs ONLY the archive - the parent can never gain a duplicate block
  (belt-and-braces: an existing block for the same fork is never re-appended)
- the parent block is broadcast only after the durable save confirms
- _save_slot_to_history folds slot._merged into closed on periodic saves
  so a resumed merged fork cannot be revived as an open tab

Implements the maintainer-approved design on issue kirodotdev#3816: visible block,
archive after merge, summary only, append at tail with gap note.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancement: merge-back a forked session (compact fork and fold summary into parent)

3 participants