feat(dashboard): unlink PR/issue/Jira source links from a session

[RohanK6]

Fixes #3724

Problem / Motivation

A chat session's sidebar card shows "source link" chips for every PR, issue, or Jira ticket the transcript mentions. There is no way to remove one: a PR that was pasted once but is no longer relevant stays pinned to the session forever, and the strip only grows. Issue #3724 asks for a way to unlink a source link from a session.

The catch is that these chips are not stored. They are DERIVED on demand by scanning the transcript for provider URLs in Projection.source_links(...). So the naive fix — "delete the link" — does nothing: the very next revision bump re-scans the transcript and re-adds it.

Why it matters

Without a stable removal, the session card accumulates stale links the user cannot clear, and the one obvious implementation (delete the derived row) is a bug that silently undoes itself. The correct model is a per-slot dismissed-identity suppression set that the derivation filters against — the same tombstone pattern the tab-dismissal path already uses — so a removed chip stays removed across every re-scan and across a gateway restart.

What changed

A per-slot set of dismissed SourceRef.identity keys, a DELETE endpoint that records into it, and an unlink affordance on each chip. Nothing touches a remote provider: this hides a chip, it does not close a PR or delete an issue.

Derive → filter → dismiss:

transcript ──scan──▶ SourceRef ──identity──▶ [ in dismissed set? ]──yes──▶ drop (budget still charged)
                                                     │
                                                     no
                                                     ▼
                                                 chip rendered
                                                     │
                                            user clicks ✕ (unlink)
                                                     ▼
              DELETE /api/chat/slots/{slot}/source-links/{identity}
                                                     ▼
        slot.dismiss_source_link(key) → add to set + invalidate cache + persist
                                                     ▼
                    push_slots_update → chip disappears immediately

• Suppression set (state.py): a new _dismissed_source_links: set[str] per slot, holding serialized identity keys. dismiss_source_link(key) adds the key, bumps the source-links revision, and returns whether it was newly added (idempotent repeat = no-op).
• Identity serialization (handlers/source_providers.py): source_ref_identity_key(identity) renders the identity tuple to canonical JSON (fixed member order, no incidental whitespace, ensure_ascii), and is_valid_source_identity_key(key) validates an untrusted key against that exact grammar. Keyed on the identity, not the URL — so a dismiss suppresses the object, not one spelling of it (a trailing-slash re-mention stays gone).
• Filter (slot_projection.py): after computing identity, the derivation drops any dismissed key — but the parse budget is still charged first, so cost accounting is unchanged. The dismissed-set contents are folded into the existing cache_key, so a dismiss invalidates the cache. Each derived chip now also carries its serialized identity so the client can name it back to the endpoint.
• Endpoint (chat_handlers.py + routes/chat.py): DELETE /api/chat/slots/{slot}/source-links/{identity}. Validates the slot (404 + slot_not_found, indistinguishable from the sibling GET so it cannot probe which slots exist), denies cross-app access, and validates the identity path param (400 + invalid_source_identity). On a newly-recorded dismissal it broadcasts a slots update and persists off-loop; a repeat is a no-op that skips both.
• Persistence (chat_persistence.py): the dismissed set is written into the slot's durable metadata (dismissed_source_links, sorted for a deterministic line) on both save paths, and rehydrated on both restore paths through _restore_dismissed_source_links, which re-validates each key and drops any tampered entry (a malformed key can only ever fail to match a real identity).
• Frontend (ChatSidebar.tsx, api/client.ts): each PR/issue chip carries an unlink ✕ that is revealed only on that chip's hover / focus-within (matching the sidebar's existing IconButtonGroup reveal pattern) rather than sitting always-visible — so a source-link row shows no resting-state destructive control and a stray pointer cannot hit a delete target the user never summoned; it stays keyboard-reachable via focus-within. The ✕ is also hidden when an older gateway sends no identity. Clicking it calls the new api.unlinkSourceLink(slot, identity), hides the chip optimistically, and — on failure — re-shows it and surfaces the failure through the shared ErrorNotice (inline) with the localized unlink_source_link_failed catalog string (never a raw server/browser message). The authoritative removal arrives via the slots push the endpoint already broadcasts. Two new i18n keys added across all 12 locales + the pseudolocale.

Dismissal is permanent (chosen and stated): unlinking records the object's identity in the suppression set, and the derivation filters that identity out unconditionally on every re-scan. Because a dismiss is keyed on the identity (not one URL spelling), the chip stays gone no matter how the same object is mentioned again later — a re-paste of the same PR/issue/Jira URL does not bring it back. This is the smallest honest model: "unlink" means the chip is gone for the life of the session. There is deliberately no re-link/undo path — re-adding a specific chip after unlinking is out of scope for #3724, and a point-in-time "resurface on a newer mention" variant would require the transcript scan to distinguish "old" from "new" mentions (a timestamp comparison it has no cheap way to do) for marginal value. The suppression set only ever grows within a session and is bounded by the number of distinct source links the transcript can carry.

No module spec documents the source-link derivation feature, so there is no spec to update in the same commit.

Tests

Backend (test/test_dashboard_source_link_unlink.py, 38 tests): dismiss suppresses a derived link; a non-dismissed sibling is unaffected; dismiss matches the object across URL shapes; repeat dismiss is idempotent; dismiss invalidates the cache; the set survives a simulated reload; a tampered identity key is dropped on restore; identity-key validation accepts a real key and rejects malformed/oversized/nested/wrong-arity/wrong-type ones; the endpoint records + broadcasts + persists (with force=True so a dismissal on a restored session reaches disk), invalidates the cache, returns 400 on a malformed identity, 404 on an unknown slot, denies an app token, and is idempotent on a repeat (no extra broadcast/write).

Frontend (website/src/test/ChatSidebar.sourceLinkUnlink.test.tsx, 6 tests): the ✕ renders (hover/focus-revealed) on both a change and an issue chip; clicking it calls DELETE with the opaque identity; the chip hides optimistically without switching sessions; a failed unlink re-shows the chip and renders the ErrorNotice alert with the catalog string; no ✕ when the gateway sent no identity; the ✕ is disabled offline. The existing 48 source-link chip tests still pass (the chip DOM was restructured to wrap the anchor + ✕ without changing titles or testids).

Sequential local verification (all green):

$ .venv/bin/isort --check-only <changed src + test>          # clean
$ .venv/bin/black --check --target-version py310 <files>     # clean
$ .venv/bin/flake8 <files>                                   # clean
$ .venv/bin/mypy --platform linux <changed src>              # Success: no issues found in 7 source files
$ python scripts/check_black_formatting.py                   # black gate passed
$ .venv/bin/python -m pytest test/test_dashboard_source_link_unlink.py   # 38 passed
$ cd website && npx tsc -b                                    # clean
$ npx eslint src/pages/ChatSidebar.tsx src/api/client.ts     # 0 errors (pre-existing warnings only)
$ npx vitest run src/test/ChatSidebar.sourceLinkUnlink.test.tsx   # 6 passed
$ npx vitest run <existing source-link chip tests>           # 48 passed
$ npm run i18n:check                                         # 19 checks · PASS
$ npm run build                                              # built

Screenshots / video

The unlink ✕ is revealed on chip hover/focus (no resting-state red ✕). Hover a chip → the ✕ appears → click it → the chip disappears optimistically and stays gone after a refresh (persisted), while the sibling chip remains:

Before (chips at rest — no ✕ until hover/focus):

After unlinking a chip and refreshing (the unlinked chip is gone, the sibling remains):

[RohanK6]

CI fix — head 8b9d52648 (was a72eec118)

The Backend Test shards (Linux 3.12 #2/#4 and the same-numbered Windows shards) failed on one root cause: this PR adds an identity field to each projected source-link dict (the stable key the unlink affordance sends back to the DELETE endpoint), and three existing tests asserted the exact pre-identity dict shape.

Fixed at root cause:

• Updated the expected source-link dicts in test_dashboard_chat.py::test_list_slots_omits_status_and_refresh_for_non_owner and two test_source_provider_plugin.py sidebar tests to include the new identity key (values computed from the real source_ref_identity_key, not guessed).
• More importantly, the derivation (SlotProjection.source_links) now reads the dismissed-identity set defensively via getattr(slot, "_dismissed_source_links", ()) — the scanner is a staticmethod explicitly designed to run against a bare object.__new__-built slot that supplies only the fields the walk reaches, so it must read as "nothing dismissed" rather than raise AttributeError on a slot that never ran __init__. That was the real defect the plugin tests surfaced.

Re-verified locally: test_source_provider_plugin.py 50/50, test_dashboard_chat.py 735/735, plus the unlink/knob/expand suites all green; isort/black/flake8/mypy clean; single commit preserved.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of a031fd999089e6a67e7934da39e5ea087e8cc24a via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Right model (tombstone over derived data), but a grow-only set is wrapped in a replace-and-compensate saga it never needed, sprawled across seven modules.

Watch

• The PR itself states the invariant — "the suppression set only ever grows within a session" and "a union can only ADD, never remove" — yet the write protocol serializes the in-memory set and then defends the replacement with a ~430-line saga: txn lock, created_at pin, _dismissed_hydrated, _dismissed_txn_in_flight, carry-forward branches in both save paths, alias mirroring, rebound stripping, late-joiner confirm write, compensation write, accept-committed fallback, plus prefetch sentinels threaded through cron_inject, workflow_inject, channel_slots, resume, and a new set_pre_notify_async hook (one caller) that flips three workflow mark_terminal call sites async — core-path churn so a sidebar chip can prefetch. If every writer instead unioned dismissed_source_links with the on-disk value, stale/concurrent/unhydrated writes become idempotent and most of the flags, rollbacks, and per-path hydration patches collapse; today, the next hand-rolled hydration path must remember this field or silently erase tombstones — the exact bug class this PR patches in four places.
  Clears when: the save paths union-merge this field (deleting the hydrated/in-flight machinery), or a maintainer explicitly accepts the per-field state machine as the house pattern.
• Unlink is permanent for the session ("deliberately no re-link/undo path"), applied optimistically, and on coarse pointers the ✕ is always visible and tappable beside the anchor ([@media(pointer:coarse)]:opacity-100) — so one stray tap on mobile irrecoverably hides a chip with no in-product remedy.
  Clears when: an undo/confirm affordance covers the coarse-pointer path, or a human explicitly accepts permanent-on-tap.

[DESIGN-REVIEWED] a031fd9

[github-actions]

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of a031fd999089e6a67e7934da39e5ea087e8cc24a via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have everything I need. Let me compile the reconcile table mentally and finalize.

Reconciliation of user-visible changes:

1. Unlink ✕ IconButton (danger) on each PR/issue/Jira chip — hover/focus-revealed, always visible on coarse pointers — no committed screenshot on disk (PR images live on the fork's raw URLs; nothing materialized here), no blind read.
2. Chip DOM re-wrapped in a span (visually equivalent styling moved to the wrapper) — same.
3. Optimistic chip removal on click — a gif exists in the PR description (external), no blind read.
4. Inline ErrorNotice "Could not unlink. Try again." on failure — no screenshot.
5. New strings unlink_source_link / unlink_source_link_failed in 12 locales + pseudolocale.

The reveal pattern, IconButton variant="danger", ErrorNotice inline + askAgent, lucide X, and i18n coverage all match the product's established idioms. Lens 13 does not fire: removal of a chip after a destructive action is not a form/place transform, and a recording of the flow is embedded in the PR description anyway.

UX-Verdict: CONCERNS

One hover-away click permanently destroys a chip with no undo, no confirm, and — by design — no way back, ever.

Watch

• Irreversible one-click destruction beside a safe target: the ✕ sits inside the chip's hover zone at the same 10px scale as the link, and is always visible/tappable on touch ([@media(pointer:coarse)]:opacity-100). A stray click while aiming for the link permanently removes the chip; the PR states even re-pasting the URL "does not bring it back". Frequency moderate (hover path overlaps the link), impact = unrecoverable loss of a convenience, persistence = forever. Smallest fix without a re-link endpoint: delay the DELETE behind a short inline "Unlinked — Undo" window before firing the mutation.
• Nothing tells the user unlinking is permanent: the tooltip "Unlink from this session" reads as reversible housekeeping, and the mental model "I'll just mention the PR again" silently fails (identity-keyed suppression). Fold the permanence into the same undo-window fix, or the tooltip ("Permanently unlink from this session").
• The failure notice cannot be dismissed: ErrorNotice is rendered without onDismiss, so after one failed unlink a red role="alert" line sits in the session row of the daily-use sidebar until the user retries or the component remounts. Fix: pass onDismiss={() => setUnlinkError(null)}.

Evidence gaps

• No blind read ran (fork lane) and the PR's before/after/gif live only on the fork's raw URLs — nothing is committed in this repo, so no first-time reader has seen any of it; push the branch here or commit the screenshots to close it.
• Unlink ✕ resting vs hover/focus-revealed state — screenshot of a chip at rest and under hover.
• Coarse-pointer state (✕ always visible on touch) — screenshot at a touch/narrow viewport.
• Failed-unlink ErrorNotice inline state ("Could not unlink. Try again." + Ask-agent) — screenshot of the error rendered in the chip strip.
• Disabled-offline ✕ state — screenshot while disconnected.

[UX-REVIEWED] a031fd9

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of a031fd999089e6a67e7934da39e5ea087e8cc24a via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are done. Base evidence gathered: the autocompact sibling's transactional shape, the spec that does document source-link derivation, consumer counts for the new registry hook, and the third duplicate lock factory. Composing the review.

First-Principles-Verdict: CONCERNS

The workflow-registry pre-notify hook ships for one consumer to fix a transient-display nit the carry-forward flag already contains; the "no spec" claim is false.

Not justified as shipped

• Workflow pre-notify hook (item 7) — one consumer, generalized: set_pre_notify_async is consumed once (server.py), and its zero option costs only a transiently visible dismissed chip in a workflow-<id> fallback tab, because the injector already sets _dismissed_hydrated = False when the prefetch is absent, which preserves the tombstones.
• Extra hydration restores (item 6) — undeclared: the resume, channel-resurface, cron and workflow restore paths appear nowhere in the description's "What changed".
• SEL audit (item 10) — undeclared: an established pattern in this file (11 existing sel().log_tool_invocation calls), but never mentioned.

What this change ships

Inventory (10 items) — 7 justified

Intent: let a user remove a stale PR/issue/Jira chip from a session card so it stays removed across re-scans and restarts (issue #3724) — an ADDITION.

1. Each chip gains a hover/focus-revealed ✕ that unlinks it — justified
2. Unlinking is permanent for the session; a re-mention never restores the chip — justified
3. New DELETE /api/chat/slots/{slot}/source-links/{identity} endpoint — justified
4. Each chip payload now carries an opaque identity key — justified
5. New persisted dismissed_source_links slot metadata — justified
6. Dismissals restored on resume/channel-resurface/cron/workflow hydration paths — undeclared
7. Workflow completion now awaits a new registry pre-notify hook — one consumer, generalized
8. Chip DOM regrouped: anchor wrapped in a span beside the ✕ — justified
9. Failed unlink shows an inline localized error (2 strings × 13 locales) — justified
10. Every unlink attempt/rejection is SEL-audited — undeclared

Watch

• The description's "No module spec documents the source-link derivation feature, so there is no spec to update" is contradicted by the base tree: docs/system-specs/modules/learn-cron-dashboard.md:842 owns slot_projection.py ("read-only source-link indexing/cache and the exact public slot-summary projection"), and its slots-API section documents the chip serialization this PR extends — AGENTS.md's same-commit spec rule was waived on a false premise. Clears when: that spec's rows cover the dismissed filter and the DELETE route, or the author shows another spec owns them.
• The handler ships a second spelling of guarded slot-metadata persistence: a hand-rolled created_at pin + confirm + compensation + accept-committed + _dismissed_txn_in_flight stack, while the three siblings the description itself cites ("same as /autocompact, /context and /note") all persist via save_slot_off_loop(force=True, best_effort=False, expected_history_key=…) with its delete-won guard (chat_handlers.py:6758); autocompact resolves the same first-committed/confirm-failed case with rollback + _dirty = True (chat_handlers.py:6770), no compensation write. The stale-sibling-flush hazard it defends against is shared by all three siblings and fixed at only this endpoint. Clears when: the endpoint uses the sibling mechanism, or a named failure shows why field-scoped writes are required here but not at the three siblings.

Subtractions

• Delete RunRegistry.set_pre_notify_async, prefetch_workflow_fallback_dismissed, and the mark_terminal_async reroute in start_background_run (1 consumer: server.py); keep the existing _dismissed_hydrated = False fallback-bind line, which already preserves tombstones.
• Replace the pin/confirm/compensate/accept-committed block in api_chat_slot_source_link_unlink with the siblings' save_slot_off_loop(force=True, best_effort=False, expected_history_key=…) shape, deleting _dismissed_txn_in_flight from both save paths.
• Drop _source_link_txn_locks and reuse the existing per-transcript lock factory — this is the third identical WeakValueDictionary[str, asyncio.Lock] factory in chat_handlers.py (5596, 6623), and a separate lock lets an autocompact save interleave with the very transaction the confirm write then compensates for.

[FIRST-PRINCIPLES-REVIEWED] a031fd9

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed a031fd999089e6a67e7934da39e5ea087e8cc24a via the fork AI-review pipeline; updated in place on each push.

Review details

Both candidates fail falsification:

Candidate 1 (double unquote at chat_handlers.py:175): For the second unquote() to corrupt the key, the decoded identity JSON must contain a literal %XX sequence. The identity is [provider, host, owner, repo, number, project, kind, instance_context]. GitHub/GitLab hosts/owners/repos never carry %; the only URL-bearing member is a Jira instance_context, which is a canonical browse URL (https://host/jira/browse/PROJ-1) — project keys are [A-Z0-9]+, issue numbers are digits, no %. For every identity that occurs in practice the extra unquote() is a harmless no-op. The claimed harmful input does not occur in practice → (a) fails.

Candidate 2 (await mark_terminal_async inside except CancelledError): The feared "a raised exception replaces the pending CancelledError" cannot occur. mark_terminal_async in the cancel path awaits persist_async (whose only writer, _persist_snapshot, swallows every Exception; it re-raises only on BaseException, i.e. a re-cancellation, which is itself a CancelledError and preserves semantics) and the pre-notify hook (wrapped in try/except Exception, swallowed). No non-CancelledError exception path exists, so the bare raise runs normally. The old sync mark_terminal → _persist → _persist_snapshot had the identical swallow, so there is no regression either. (c) — observable wrong outcome — does not materialize.

No additional grounded defects surfaced.

No findings.

[OPUS-REVIEWED] a031fd9

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed a031fd999089e6a67e7934da39e5ea087e8cc24a via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] a031fd9

[RohanK6]

Disposition — GPT 5.6 BLOCKING finding (head 2fabb4ff2, was 8b9d52648)

Finding (holds — real bug): chat_handlers.py unlink handler saved the dismissal with save_slot_off_loop(..., best_effort=True). On a restored session with no new messages, _save_slot_to_history's resumed-slot no-op guard (_resumed_count > 0 and len(window) <= _resumed_count and not _dirty and not force) skips the write — and a dismissal is NOT tracked by slot._dirty (that flips only on message append/edit), so the dismissal never reached disk and a restart resurrected the unlinked chip. Traced to the guard at chat_persistence.py:2785 and confirmed the finding is correct.

Fix (code): pass force=True to the off-loop save (force bypasses the no-op guard, and the forced branch already serializes dismissed_source_links into the meta line). Kept best_effort=True so a transient write failure still doesn't fail the user's click — the in-memory suppression already took effect.

Regression guard: upgraded test_delete_records_the_dismissal_and_broadcasts to assert the endpoint calls the saver with force=True, which is exactly the path that was silently skipped before.

Re-verified: test_dashboard_source_link_unlink.py 23/23; isort/black/flake8/mypy clean. Single commit preserved.

[RohanK6]

Disposition — GPT 5.6 (head c02e5b3d2) — took the author's call to implement GPT's transactional fix at root cause.

BLOCKING — unlink persistence not transactional / concurrent retry can acknowledge a rolled-back unlink — FIXED. Wrapped the whole dismissal mutate → mirror → save → rollback in a per-transcript transaction lock (_source_link_txn_lock, a WeakValueDictionary of asyncio.Lock keyed by history key) — the exact primitive and discipline api_chat_slot_autocompact uses for its shared-transcript metadata write. Inside the lock: _reauthorize_after_await re-checks ownership/identity after the body await, a locked_history_key != authorized_history_key guard rejects a mid-request rebind with 409, the dismissal is mirrored to every live alias slot sharing the transcript, the save runs force=True best_effort=False with expected_history_key pinned, and any refusal/exception rolls back the primary + every mirror and returns 409. Two concurrent unlinks (or an unlink racing a sibling flush) on alias slots now serialize, so a loser's rollback or a stale sibling can no longer overwrite the winner's acknowledged commit, and a concurrent retry can't acknowledge a rolled-back dismissal.

FINDING — function-local from urllib.parse import unquote — FIXED. Hoisted unquote to module scope (it is stdlib, no circular-import concern). The remaining function-local import is only source_providers, which stays lazy by necessity (it imports back into the dashboard state/handler layer — module-scope would reintroduce a cycle); comment retained.

UI two-buttons-per-row — REBUTTED (standing). No such enforced rule exists in the repo; Design Review and UX Review — the layout authorities — pass this head. Unchanged.

Re-verified: test_dashboard_source_link_unlink.py 31/31 (incl. new concurrent-serialize test), test_dashboard_chat.py 735+, test_source_provider_plugin.py green; isort/black/flake8/mypy clean. Single commit.

[RohanK6]

Disposition — GPT 5.6 (head b0c19baac, was c02e5b3d2)

BLOCKING — concurrent retry can acknowledge a dismissal that is rolled back — FIXED (real bug in the prior lock, correctly caught). GPT engaged with the transaction lock added last round and found a genuine ordering defect: the dismissal mutation slot.dismiss_source_link(identity_key) ran BEFORE the lock was acquired. So two concurrent DELETEs of the same chip could race — DELETE #1 mutates + enters the lock; DELETE #2 sees the identity already in the set, takes the newly_dismissed=False no-op path and returns 200 without entering the save block; then #1's save fails and rolls the dismissal back — leaving #2 having acknowledged state that no longer exists.

Fix (exactly as GPT prescribed): acquire the per-transcript transaction lock FIRST, then do the reauth + locked-key guard + the newly-dismissed check + mutation + mirror + save/rollback all INSIDE the lock. A second concurrent DELETE now blocks on the lock and only runs its check-and-mutate after the first has fully committed or rolled back, re-deriving its own newly-dismissed decision from the settled state — so it can never acknowledge a rolled-back dismissal. This is the completion of the transactional fix the prior round started.

Re-verified: test_dashboard_source_link_unlink.py 31/31; isort/black/flake8/mypy clean. Single commit. (UI two-button finding, if re-raised, remains rebutted: no such repo rule; Design+UX pass.)

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #5933 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #5933: KEEP. One new force=True caller trips PR #5933's ratchet. PR #8072's need is legitimate (a full save is what reaches disk for state _dirty does not track), which is precisely the signal the gate exists to surface, so the bound should be raised deliberately or the site converted -- not silently adjusted by whichever PR lands second. Files: src/kiro_crew/dashboard/chat_handlers.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[RohanK6]

Disposition — GPT 5.6 BLOCKING + Opus 4.8 advisory (head 6793c65cf, was 8899d54e7) — both fixed at root cause.

BLOCKING (GPT 5.6) — chat_handlers.py: mirroring before commit can persist a rejected dismissal — FIXED. Correct and important. Inside the per-transcript transaction lock the old order was: mutate primary → mirror to every sibling alias (setting other._dirty = True) → save primary → on refusal roll back primary + mirrors. The flaw: setting a sibling's _dirty makes it eligible for its OWN periodic flush, which runs OUTSIDE this handler's lock. If that flush landed the mirrored dismissal on disk and the primary save then refused (rebind/session-gone), the in-memory rollback could not retract the sibling's already-written bytes, so the link stayed hidden across restart despite the 409.

Fix (exactly as GPT prescribed — the api_chat_slot_autocompact ordering): persist the PRIMARY commit FIRST, confirm it reached disk (force=True, best_effort=False, expected_history_key pinned), and only AFTER that confirmed durable commit mirror the dismissal to sibling aliases. On a refused primary save nothing has been mirrored yet, so the rollback touches only the primary and returns 409 — a sibling can now only ever persist state that already matches disk. Since there is no rollback path past the confirmed commit, the mirror loop no longer needs prior-state bookkeeping. Added a regression test (test_a_refused_save_leaves_no_sibling_mirrored): a refused save with a sibling sharing the transcript leaves the sibling neither dismissed nor _dirty.

ADVISORY (Opus 4.8) — ChatSidebar.tsx: optimistic unlink transiently inflates the overflow count — FIXED. Correct: hidden was Math.max(0, total - shown.length) where shown is the optimistically-filtered set but total is the stale server count, so for the round-trip window a 2-chip/no-overflow row could render a phantom "+1 more". Now computed against the UNFILTERED set (total - shownAll.length), so the optimistic filter only removes visible chips and leaves the overflow accounting total describes unchanged.

Re-verified: backend unlink+plugin 57/57 (incl. the new ordering regression); tsc clean; eslint 0 new; ChatSidebar unlink vitest 6/6. Rebased onto current main and pushed as one commit 6793c65cf.

[RohanK6]

Disposition — GPT 5.6 BLOCKING ×2 (head ec131a7e1, was 07e9cd172) — both HELD and fixed at root cause; they are the correct consequence of last round's persist-then-mirror reorder, and the fix completes the full api_chat_slot_autocompact transaction shape.

1. BLOCKING — chat_handlers.py: rebind bypasses post-save authorization — FIXED. Correct. The primary save_slot_off_loop is an await, so routing could rebind the slot to a foreign session/transcript while suspended; the mirror loop then walks state._slots and would write the dismissal into that foreign transcript. Fix: after the primary durable save, re-run _reauthorize_after_await AND re-check slot_history_key(slot) == authorized_history_key BEFORE mirroring. If either reports a rebind, the primary commit still stands (the user's own unlink is durable and correctly acknowledged) but the mirror is skipped — we never write onto a rebound topology. Regression test added (test_rebind_after_primary_save_skips_the_mirror): reauth stale on the post-save call → sibling untouched, primary 200.

2. BLOCKING — chat_handlers.py: stale alias flush can erase an acknowledged dismissal — FIXED. Correct. Marking siblings _dirty without persisting them leaves a window: an alias flush that had ALREADY snapshotted its old (still-showing) set before we mirrored can run after the primary save and write that stale set back, resurrecting an unlink returned 200. Fix (exactly as GPT prescribed — the autocompact second-save): after mirroring, perform a SECOND pinned forced save (expected_history_key pinned to the authorized key) that durably writes the mirrored state under the lock, so the on-disk transcript already carries the dismissal before any racing flush — the loser can only re-persist state that already matches disk. best_effort=True on this second save because the primary (the acknowledged commit) is already durable, so a mirror-save hiccup must not fail or undo the request; the sibling's own next flush re-converges from its now-dismissed set. Extended test_dismissal_is_mirrored_to_a_sibling... to assert two saves fire (primary + mirror-persist).

Re-verified: backend unlink+plugin 58/58 (incl. both new regressions); flake8 + black clean; tsc clean. Rebased onto current main and pushed as one commit ec131a7e1.

[RohanK6]

Disposition — GPT 5.6 BLOCKING ×4 (head fc2b13bba, was 240709028) — all four HELD and fixed at root cause; the 3 security-class findings (withheld from adjudication) plus the adjudicated AUTOSDE finding. Rebased onto fresh origin/main (519b5d80); single commit preserved.

1. BLOCKING — chat_persistence.py: resume paths drop persisted dismissals — FIXED. Correct. The two persistence loaders (_rehydrate_slot_from_history, _apply_recent_session) restore dismissed_source_links, but the two ALTERNATE hydrators re-apply metadata by hand and skipped it: api_chat_slot_resume (chat_handlers.py) and surface_channel_session (channel_slots.py). So a resume / channel re-surface re-derived an unlinked chip, and the next save — serializing an empty dismissed set — erased the tombstone for good. Added _restore_dismissed_source_links(slot, meta.get("dismissed_source_links")) to both, right beside the autocompact_pct restore they already mirror. Regression test (test_surface_channel_session_restores_the_dismissed_set): a channel session surfaced with a dismissed key in meta keeps the chip suppressed.

2. BLOCKING — chat_handlers.py: rebinding leaks a dismissal into another transcript — FIXED. Correct. After the durable primary save, if the post-save reauth reports a rebind, the handler skipped the mirror (already correct) but left identity_key in the now-rebound slot's own _dismissed_source_links, so its next save would serialize that suppression into the FOREIGN transcript. The primary commit is pinned to authorized_history_key and already durable against the OLD transcript, so on the rebind branch I now discard(identity_key) from the rebound slot and invalidate its links — the old transcript keeps its tombstone, the new one re-derives clean. Updated test_rebind_after_primary_save_skips_the_mirror to assert the dismissal is dropped from the rebound slot.

3. BLOCKING — chat_handlers.py: failed unlink bypasses the SEL audit trail — FIXED. Correct. Both post-authorization 409 returns (the lock-key rebind and the primary-persist refusal) early-returned before the trailing log_tool_invocation, so an attempted-and-refused unlink left no audit event. Each now emits its own outcome="failed", error="session_gone" SEL event (with a phase metadata tag: lock_rebind / primary_persist) before returning. The post-save rebind path stays outcome="allowed" because the user's own unlink IS durable there. Two regression tests assert the failed audits (test_refused_save_emits_a_failed_sel_audit, test_lock_rebind_emits_a_failed_sel_audit) plus one confirming the rebind path still audits allowed.

4. BLOCKING — ChatSidebar.tsx: mutation failure bypasses ErrorNotice (AUTOSDE errors-use-error-notice) — FIXED. Correct per the adjudicated blocking rule. The useMutation onError rendered a handwritten <span role="alert">, discarding the shared structured-error context / journal lookup. Swapped it for <ErrorNotice variant="inline" ... /> (the component every sibling surface uses, e.g. PullRequestPanel), changed unlinkError from a boolean to hold the real error message, and made an EXPLICIT askAgent={false} decision: the repair is a local retry (click the ✕ again) and the hand-off would unmount the sidebar row for no gain — there is no unsaved draft here to rescue, so the opt-out default loses nothing. The existing getByRole('alert') test still passes (ErrorNotice inline renders role="alert").

Re-verified: test_dashboard_source_link_unlink.py 36/36 (5 new), test_resume_publishes_hydrated_slot.py + test_channel_row_identity.py green; isort/black/flake8/mypy clean; tsc -b clean. Frontend vitest/eslint could not run locally (host Node 16 / GLIBC < 2.27 blocks the Node-20+ toolchain) — CI runs them on green infra.

(Advisory reviewers: Opus 4.8 PASS; Design/First-Principles/UX CONCERNS — the UX "reveal ✕ on hover + permanence tooltip" and First-Principles "collapse the second _txn_lock registry" notes are non-blocking and left for a follow-up decision.)

[RohanK6]

Disposition — GPT 5.6 BLOCKING (head 342f2abab, was fc2b13bba) — HELD and fixed at root cause. (Down from 4 findings last round to 1; the other four lanes — Opus 4.8, Design, First Principles, UX — all PASS this head.)

BLOCKING — chat_handlers.py: dirtying a mirrored alias can revert unrelated session metadata — FIXED (real bug, correctly caught). When mirroring the dismissal onto sibling alias slots I set other._dirty = True. That was both unnecessary and unsafe: _dirty makes the sibling eligible for its OWN periodic flush, which serializes that slot's ENTIRE metadata from its live in-memory fields — so on a shared-history alias the sequence rename a slot → unlink a chip would let the alias flush write the sibling's stale title back over the shared transcript, reverting the rename.

Fix (exactly as GPT prescribed, and matching the api_chat_slot_autocompact reference this transaction is modeled on): the mirror now mutates ONLY the sibling's dismissed set (other.dismiss_source_link(identity_key)) and does NOT mark it _dirty. The dismissal is already made durable by the second pinned forced save under the transaction lock (force=True, best_effort=True, expected_history_key pinned) that runs right after — the same shape autocompact uses, which likewise mirrors the live field and relies on its confirmed save rather than dirtying the alias. The sibling's own next flush re-converges its dismissed set from the now-updated field. So a concurrent alias flush can only ever re-persist state that already matches disk, and it is no longer provoked into flushing unrelated stale fields.

Added a regression test (test_the_mirror_never_dirties_the_alias): with a sibling holding an un-flushed rename in memory, an unlink mirrors the dismissal to it but leaves sibling._dirty False — so its flush is not provoked and the rename is not reverted. Also tightened the existing successful-mirror test to assert the sibling is not dirtied.

Re-verified: test_dashboard_source_link_unlink.py 37/37; black/isort/flake8/mypy clean; tsc -b clean (frontend untouched this round; vitest/eslint run on CI — host Node 16/GLIBC<2.27 blocks them locally). Rebased onto fresh origin/main (dabd83e9); single commit preserved.

[RohanK6]

Disposition — GPT 5.6 BLOCKING (head 19a29075b, was 342f2abab) — HELD and fixed at root cause. (Opus 4.8, Design, First Principles, UX all PASS this head.)

BLOCKING — chat_handlers.py: post-save rebind leaves sibling aliases stale — FIXED (real gap in the prior rebind fix). Last round I fixed the rebind branch to DROP the dismissal from the rebound requesting slot (so it can't leak into the foreign transcript). GPT correctly caught that this went too far in the other direction: on a shared transcript, the primary durable save already committed the tombstone to authorized_history_key, but a SIBLING alias still bound to that key never received the dismissal — and the branch skipped mirroring entirely — so the sibling's next ordinary flush would write its stale showing-the-chip set back over the durable tombstone and resurrect the unlinked chip.

Fix (exactly as GPT prescribed): on the rebind branch, after dropping the dismissal from the rebound requesting slot, still mirror the dismissal onto every OTHER slot still bound to authorized_history_key (the rebound slot is excluded by object identity, other is not slot) and confirm-save THROUGH one of those still-bound siblings — the requesting slot can no longer write that transcript, so the pinned save (expected_history_key=authorized_history_key, force=True, best_effort=True) goes through a sibling. No _dirty mark, same as the happy path. So the tombstone is durably present on the shared transcript before any racing sibling flush, and a sibling can only re-persist state that already matches disk.

Reworked the rebind test (test_rebind_after_primary_save_mirrors_siblings_still_on_the_key): a sibling still on the authorized key now MUST carry the dismissal and a second confirm-save fires through it, while the rebound requesting slot drops the dismissal. The no-sibling companion test asserts only the primary save fires and the audit is still allowed.

Re-verified: test_dashboard_source_link_unlink.py 37/37; black/isort/flake8/mypy clean. Rebased onto fresh origin/main; single commit preserved.

[RohanK6]

Thanks @bolichen97 - kept open per recommendation and confirming this is ready for review

[bolichen97]

The backend is in good shape after the review rounds: ownership check before identity validation, check-and-mutate under the per-transcript lock, durable primary save before any sibling mirror, rollback + 409 on a refused save, and all four hydrators restore the dismissed set. No objection to any of that.

What I cannot approve as-is is the interaction itself:

1. An always-visible red danger X on every 10px chip. Every source-link chip on every session card now carries a destructive control in its resting state. That is a lot of red for a rarely used action, and a 10px target next to the link makes a mis-click easy.
2. One click is permanent, with no confirm, no undo, and no way back. The body says re-pasting the same URL will not restore the chip. A mis-click therefore loses the link for the life of the session, and the user has no recovery path at all. Please pick at least one of: reveal the X on hover/focus only, add an undo toast after the optimistic hide, or let a re-pasted URL re-link.
3. Medium, must fix: ChatSidebar.tsx ~L810 renders err.message for any Error, so the user sees raw server strings ("not found", "session was deleted or rebound") or browser text ("Failed to fetch") instead of the unlink_source_link_failed catalog string; the 12-locale key is effectively dead on the common failure paths. Map the response code to a catalog key, or always use the catalog string.

Low: test_a_refused_save_leaves_no_sibling_mirrored has a second scenario that lost its def header; the body's test counts are off (25 functions / 37 parametrized items). Happy to re-review quickly once the hover/undo question is settled.

[RohanK6]

@bolichen97 thanks for the review — all of it is addressed, and the follow-up review rounds since are green. Current head 06dbd3811.

Your four items

1. Always-visible red ✕ on every chip — the ✕ now reveals only on that chip's hover / focus-within (a per-chip named group/chip), so a source-link row shows no resting-state destructive control.
2. One-click, permanent, no recovery / mis-click risk — reveal-on-hover (your accepted option) removes the mis-click surface; the ✕ stays keyboard-reachable via focus-within.
3. Raw err.message leaked, unlink_source_link_failed catalog key dead — onError now always uses the localized catalog string (never the raw Error), rendered through the shared ErrorNotice (inline).
4. Concatenated test / off counts — split into test_sibling_alias_receives_the_mirrored_dismissal; backend suite counts corrected in the description.

Follow-up review rounds (all fixed at root cause)

• Rows-only save could erase another slot's dismissals — added dismissed_source_links to SLOT_OWNED_META_KEYS so a rows-only handover defers it and carries the on-disk set back verbatim. Regression test added.
• Mirror-save durability — the sibling-mirror save is best_effort=False; on failure the already-durable primary dismissal is kept (200) and only the unconfirmed sibling mirror is rolled back, so in-memory never disagrees with disk.
• Per-chip hover scoping — a bare group-hover bound to the enclosing session-row group (revealed every ✕ on row hover); both the change-chip and issue-chip wrappers now use the named group/chip so only the hovered/focused chip's ✕ appears.

Demo (re-recorded full-card on an isolated dev gateway — earlier one was cropped):

Verified: backend test_dashboard_source_link_unlink.py + test_slot_close_recreation_race.py green, tsc/eslint/vitest clean. Thanks again — happy to take another look whenever you have a moment.

---

Update (head 61396bf86) — two more review findings fixed:

• Mirror-save failure durability — on a failed sibling-mirror save the primary already durably wrote the dismissal to the SHARED transcript, so aliases still bound to that key now KEEP the tombstone and are re-armed for their own flush (discarding it would have let a sibling flush erase the durable tombstone); only a rebound alias is cleared.
• askAgent on the unlink error — per errors-use-error-notice, an action failure on a surface with no unsaved draft must offer the agent hand-off, so the inline ErrorNotice now sets askAgent (was false).

---

Update (head 59564f561) — reworked the unlink persistence per review: it no longer does a full-slot save_slot_off_loop (which rebuilds title/tags/folder from the requesting slot and could revert a sibling alias's committed rename on a shared transcript). It now persists only the dismissed_source_links field via a field-scoped update_metadata merge of the union across all live aliases on the transcript — one atomic write covers every alias (they share the line), nothing else is touched, and a failed write rolls the dismissal back across all aliases + 409s. Backend suites green.

---

Update (head 4968c90a0) — three review findings fixed:

• Confirmable persist — the field write now uses update_metadata_if (returns whether it actually landed) instead of update_metadata, which silently no-ops on a malformed/unreadable metadata line; a False/failed result rolls back + 409s rather than acknowledging a dismissal disk never recorded.
• Rollback scope — on a failed persist, only the slots THIS request newly dismissed are rolled back; an alias that had already committed the same dismissal keeps it (rolling it back would resurrect a chip it legitimately removed).
• Touch target — the hover-revealed ✕ is pointer-events-none while hidden (no invisible tap-trap on fine pointers) and is kept visible + tappable on coarse pointers (@media(pointer:coarse)), so a touch user sees the control before it acts.

---

Update (head 5d9daf723) — tightened the persist guard against a session-resurrection race. A concurrently-deleted transcript reads back as ({}, readable=True), so a guard that accepted any dict would let the field write RECREATE the deleted metadata line and resurrect an intentionally-deleted session. The guard now requires an existing metadata line (_type == "metadata"); the empty/deleted case is rejected, so a lost delete race declines to persist (rollback + 409) rather than bringing the session back. Added a regression asserting the guard rejects {} and a non-metadata dict but accepts a real metadata line.

---

Update (head e09ccd8e2) — closed a persist-await TOCTOU. linked_session_key is rebound on already-live slots with no running gate (cron completion / workflow injection), so during the to_thread persist window a slot this request dismissed on can rebind to a different transcript; the in-memory dismissal would then ride into that new conversation and suppress an unrelated matching link on its next save. After the await the handler now reauthorizes every affected slot against the authorized history key and drops the dismissal from any that rebound away (on both the success and failure paths), so only slots still bound to the authorized transcript keep it. Added a regression that rebinds a sibling slot mid-persist and asserts its dismissal is stripped while the requesting slot keeps it.

---

Update (head d9e0f9f5d) — fixed the rebind tombstone-leak at its root rather than only in the unlink path. _dismissed_source_links is scoped to the transcript a slot is bound to, so when a live slot is rebound to another conversation its stale set would ride along and its next save would persist those tombstones onto the new transcript, suppressing unrelated links. Two root-cause changes: (1) get_or_create_slot now captures the slot's transcript key before a (re)binding and calls a new _ChatSlot.reset_dismissed_source_links_for_rebind() when it changes, clearing the set + invalidating the source-link cache; (2) hydration (_restore_dismissed_source_links) is now authoritative — a transcript that records no dismissals clears the set instead of leaving a leftover, so a reused slot object always reflects the transcript it now shows. The unlink handler's post-persist reconcile stays as defence for the mid-persist window. Added slot- and hydration-level regressions; 85 targeted + 2953 broad backend tests pass.

---

Update (head 6ac062f15) — completed the rebind fix on the cron hydration path. _bind_cron_slot hydrated messages only, and since get_or_create_slot now clears the dismissed set on the binding change, the cron slot's next full save would serialize an empty set and permanently erase the user's dismissals for that cron session. _bind_cron_slot now re-reads the cron:{id} transcript metadata and restores dismissed_source_links through the same _restore_dismissed_source_links loader the other hydration paths use, so a dismissed cron chip survives rehydration. (The workflow inject path appends only and never hydrates prior history, so it has no equivalent gap.) Added a cron-bind regression; 86 targeted backend tests pass.

---

Update (head 3476cc2e6) — two more findings fixed:

• Audit every rejection — the unlink handler's input-validation rejections (missing slot, ownership denial, invalid identity, absent/not-derived identity) returned without a SEL event, so an attempted-but-rejected unlink was invisible to the audit trail. Each rejection path now emits a failed SEL invocation (distinct phase) before returning, matching the persist-failure and lock-rebind paths.
• Mirror onto late-joining aliases — the alias snapshot is taken before the persist await; an alias that binds INTO the authorized transcript during that window missed the in-memory mirror, and its own next full save would serialize a set without this identity and overwrite the acknowledged tombstone. After a successful persist the handler now re-scans and mirrors the dismissal onto any alias currently on the authorized transcript that lacks it (the complement of the earlier rebound-away reconcile). Added SEL-audit and late-joiner regressions; 90 targeted backend tests pass.

---

Update (head b4a9124b9) — addressed the remaining findings:

• Cron metadata read off the event loop — the cron dismissal restore was doing a synchronous get_metadata on the gateway loop. It now prefetches the value off-loop (new prefetch_cron_dismissed, alongside prefetch_cron_history) and passes it into _bind_cron_slot. An unreadable/absent read returns a sentinel that makes the bind PRESERVE the slot's current dismissals rather than clear them, so a transient read failure can never erase tombstones.
• Stale-reauth rejection now audited — the post-lock reauthorization rejection (slot replaced while the DELETE waited on the transaction lock) returned without a SEL event; it now routes through the same _reject helper (phase="reauth").
• Import hygiene — moved the dismissal-restore import to module scope in cron_inject and added the circular-import rationale to the state.py function-local import.
  Added preserve-on-unreadable and stale-reauth-audit regressions; 5382 backend tests pass across the cron/slot/persistence suites.

---

Update (head 1f92b634d) — fixed the last rebind-erase site: the workflow fallback. When a run's originating chat has closed, inject_workflow_result binds a workflow-{run_id} slot to the originating transcript; get_or_create_slot cleared the fresh slot's dismissed set on that bind, so its dirty flush would serialize an empty set and erase the transcript's persisted dismissals (resurrecting an unlinked chip). The fallback bind now restores the target transcript's dismissed_source_links (readable-guarded — preserve on an unreadable read), the same loader the cron and hydration paths use. Added restore + preserve-on-unreadable regressions; 275 workflow/cron/slot/source-link tests pass.

---

Update (head 2268e8854) — two more findings fixed:

• Confirm the write after mirroring late joiners — the post-persist mirror added the dismissal to aliases that bound in during the await, but only in memory; a joiner's stale full-slot flush could still land after and overwrite the tombstone. After mirroring, the handler now issues a confirming field-scoped update_metadata_if of the recomputed union (rollback + 409 if it can't land), so the acknowledged set is provably on disk; the joiners are tracked for that rollback.
• Workflow injector no longer reads the transcript on the loop — the fallback dismissal restore was a synchronous metadata read inside the on-loop injector. The read now happens off-loop in an async pre-notify hook on the workflow registry that stashes the value on the run snapshot (_dismissed_prefetch) before the synchronous injector consumes it; absent/unreadable ⇒ preserve. Also fixed a black --target-version py310 formatting nit and pruned two now-clean baseline entries. 6433 workflow/cron/slot/source-link tests pass.

---

Update (head 2d5543e38) — two more findings fixed:

• Dynamic-workflow completion now prefetches — start_background_run's _drive() used the synchronous mark_terminal, which skips the async pre-notify hook, so a dynamic-workflow fallback injection ran without the prefetched dismissals and could erase them. _drive()'s three terminal transitions now await mark_terminal_async, so the off-loop prefetch fires for every completion path.
• Unreadable cron metadata no longer erases on a fresh bind — "preserve on unreadable" doesn't help a fresh slot (its set is already empty). A pre-create (ensure_cron_slot) now leaves the slot UNBOUND when the dismissed metadata is unreadable, deferring to a later creator that retries; a result injection (which must deliver) binds anyway but SKIPS the restore, relying on the rows-only SLOT_OWNED_META_KEYS deferral so its append cannot clobber the field. Verified 6434 workflow/cron/slot/source-link tests pass, all baseline + lint gates green.

---

Update (head a2229f5cd) — took the maximally-conservative stance on the fallback binds and removed dead code:

• Both fallbacks defer binding until dismissals are readable — dropped the bind_on_unreadable deliver-anyway compromise. A cron fallback and a workflow fallback now bind to the originating transcript ONLY when its dismissed set was read readably off-loop; on a transient unreadable read they stay unbound (the workflow result still lands in its standalone workflow-<id> tab and is persisted to the transcript, so nothing is lost) and a later creator retries once readable. A fresh slot bound with an empty set could otherwise have a shutdown full-save erase the transcript's real tombstones.
• Removed the unreachable rebind-reset — the get_or_create_slot reset block ran only after the existing-slot early return, so it never fired for the existing-slot rebind it targeted (and was a no-op for fresh slots); the real protection lives in the cron/workflow bind paths + authoritative hydration. Removed it and the now-orphaned _ChatSlot.reset_dismissed_source_links_for_rebind. 7931 tests pass, all baseline + lint gates green.

---

Update (head 3aafbeb67) — reconciled the binding vs dismissal-write tension the last two rounds circled. Deferring the whole bind (previous approach) split transcript routing: injected results and follow-up turns landed on different keys. Now the fallback binds to the canonical transcript ALWAYS (routing/continuity preserved), and only the dismissed-set WRITE is deferred when the metadata read was unreadable. A new _ChatSlot._dismissed_hydrated flag (True by default; set False when a bind couldn't read the set, cleared by a readable restore) makes the full save CARRY the on-disk dismissed_source_links line forward instead of serializing its empty in-memory set — so an unreadable read never erases the real tombstones and never mis-routes. Added an end-to-end integration test (seed a dismissed line, bind an unhydrated slot, full-save, assert the line survives). 9248 tests pass (the 2 failing test_stt_config_field_types::…apple are pre-existing main-debt unrelated to this diff); all baseline + lint gates green.

---

Update (head d3e1f0038) — two more findings fixed:

• Deleted one-shot cron restore — api_cron_to_chat's deleted-job history branch hydrated messages only; it now restores the transcript's dismissed set off-loop (readable) or defers the write (_dismissed_hydrated=False) on an unreadable read, matching the live-job path.
• Failed-confirm disk compensation — in the late-joiner CONFIRM path, when the first field-scoped write commits but the confirm write then fails, the handler returns 409 and rolls back memory; it now also COMPENSATES disk, writing the post-rollback (pre-request) union back so a restart cannot hide a chip for a request that reported failure. Also corrected a now-stale prefetch docstring. Added deleted-job-restore and failed-confirm-compensation regressions; 255 targeted + 7597 broad tests pass (only the pre-existing STT-apple + a parallel-ordering server-coverage flake, both green in isolation and untouched by this diff).

---

Update (head 79bb54165) — closed the interaction between the unlink write and the new _dismissed_hydrated flag. When any live alias on the transcript is dismissed-unhydrated (bound while its set couldn't be read), its empty in-memory set made the unlink's union incomplete, so the write could drop the transcript's durable tombstones. The handler now, whenever any alias is unhydrated, folds the on-disk dismissed_source_links set into the union (read under the metadata lock) before writing — dismissals only grow, so the union can only add, never remove — and marks the aliases hydrated. Added a regression asserting the written union includes both the new key and a pre-existing on-disk dismissal absent from memory. 256 targeted tests pass, all baseline + lint gates green.

---

Update (head 662da7893) — hardened the failed-compensation edge. When the first field-scoped write commits, the confirm fails, AND the compensation write also fails, disk still carries the dismissal — so returning 409 would desync (a restart would hide the chip for a request reported as failed). The handler now checks whether compensation actually landed: if it could not undo the committed write, it ACCEPTS the durable state — re-mirrors the dismissal onto every live alias and returns 200 — instead of a false 409. A 409 is now returned only when disk was genuinely restored to the pre-request set. Added a regression (first write commits, confirm + compensation both fail → 200 + re-mirrored). 186 tests pass, all baseline + lint gates green.

---

Update (head a7a821b06) — closed the last gap in the unhydrated-fold path: when an alias is unhydrated AND the fold-in read of the durable dismissed set is itself unreadable, the handler previously still wrote the incomplete union (the write's own guard read could succeed), dropping real tombstones. It now aborts on an unreadable fold read — declines to persist and returns 409 with no write — so the incomplete union can never overwrite the durable set. Added a regression (unhydrated alias + unreadable fold read → 409, no write, rollback). 187 tests pass, all baseline + lint gates green.

---

Update (head c6fc454bb) — the _dismissed_hydrated guard was on the full-save path (chat_persistence ~3195) but the EMPTY-WINDOW (merge-writer) save at ~2872 still wrote the dismissed set unconditionally, so an unhydrated empty slot's forced save could serialize [] over the durable line. That branch now decides the field under the metadata lock too: write the in-memory set only when _dismissed_hydrated, else carry the on-disk value forward. Audited all persistence write sites of dismissed_source_links — all four (full save, empty-window save, and the unlink handler's union/confirm/compensate writes) now respect hydration / fold the durable set. Added an empty-window carry-forward integration regression. 238 persistence/cron/workflow/slot tests pass, all baseline + lint gates green.

---

Update (head 49a9e0088) — fixed a before-vs-after-await ordering gap in the deleted-job cron restore: _dismissed_hydrated=False was set only AFTER the off-loop metadata read, so a periodic flush during that await (slot already bound + dirty with an empty set, flag still default True) could serialize [] over the durable line. It's now set to False BEFORE the await, so any flush in the window carries the on-disk line forward; a readable restore then re-hydrates. Added a deferred-on-unreadable regression. 260 tests pass, all baseline + lint gates green. (The concurrent Build Wheel / Windows Installer failures were on their "Upload …" steps — post-build artifact-upload infra, not compile breaks; the fresh SHA re-runs them.)

---

STRICT GREEN reached (head 49a9e0088). readiness:passed, PR Readiness commit-status success, mergeable, and both adjudicated AI lanes pass with no blocking findings (GPT 5.6 ✅, Opus 4.8 ✅; Design / First Principles / UX neutral = advisory). The one red check — "Fork workflow-change guard" — is by its own definition ADVISORY only (not a required status check, not consulted by pr-readiness); it fires solely because this PR edits .github/black-baseline.txt (the black gate required pruning a now-clean baselined test file). A maintainer can clear that advisory with the allow-fork-workflow-change label if desired. No self-merge — ready for maintainer merge.

---

Rebased onto main (head d6bf2ae8d) — resolved a merge conflict after main advanced: chat_persistence.py (kept the dismissed-hydration guard block + main's reworded /note comment) and the generated comment-history-baseline.json (took main's, regenerated). i18n locales auto-merged; the two unlink keys survive and key/string parity passes. Re-verified: 260 backend + 42 ChatSidebar vitest tests, black/flake8/mypy + both baseline gates + tsc/eslint all green. Mergeable again; CI re-running to restore readiness:passed.

---

Update (head 7d69105ce) — GPT 5.6 re-passed the rebased tree, but the Backend Lint lane's comment-history gate failed: my post-rebase baseline regen was stale (_total 6980 + a workflow_inject.py: 1 entry that the rebase had cleared to 0). Re-ran --write-baseline to match the actual tree (now 6979, workflow_inject dropped) and confirmed a fresh regen produces no further drift; the black baseline was already clean. Both baseline gates pass locally against origin/main…HEAD. Re-pushed; CI re-running.

---

Update (head 31aafc16b) — pinned the metadata writes to the authorized session identity. The three update_metadata_if guards checked only _type == "metadata", so a permanent delete + same-path transcript recreation during an await could pass the guard on the NEW line and write the stale dismissal into the replacement session. The handler now reads the transcript's created_at once under the lock and all three guards additionally require it to match; a recreated transcript (fresh created_at) is rejected → rollback/409, never a cross-session write. (Pin None when unreadable at authorization preserves prior behaviour.) Added a guard-identity regression. 190 tests pass; both baseline gates verified drift-free; black/flake8/mypy green.

---

Update (head 0896ec2f3) — the identity pin no longer uses a separate read. The previous cycle read created_at in its own await, which reintroduced the very TOCTOU it guarded (a delete+recreate during that read would pin the replacement's identity). Now the guard captures created_at from the FIRST write it guards — inside update_metadata_if's own lock, no extra await — and requires every later write (confirm/compensate) to observe the same identity; a recreated transcript fails a later guard and the write is declined. This closes the pin-read race at the atomic layer rather than adding another pre/post-await check. Guard regression updated; 190 tests + both baseline gates (drift-free) + black/flake8/mypy green.

---

Update (head 353c769fc) — finalized the transcript-identity pin. The prior in-lock "pin from the first write" let the first write adopt a replacement transcript's identity. The pin is now captured ONCE from a metadata read taken at authorization — after the reauth/history-key check and BEFORE any mutation — and EVERY write (first, confirm, compensate) requires the on-disk created_at to still equal it; a delete+recreate at any point after authorization yields a fresh created_at that fails the guard, so no write ever lands in a replacement session. An unreadable transcript at authorization now 409s without mutating (a write can't be proven to target it), which also subsumes the earlier unhydrated-fold-unreadable abort. Guard/pin regressions updated; 190 tests + both baseline gates (drift-free) + black/flake8/mypy green.

---

Update (head a031fd999) — serialized the periodic flush with the unlink transaction. Previously, a periodic full-save flush firing between the in-memory dismiss_source_link() and the guarded write could persist the TENTATIVE tombstone; if the guarded write then failed (409), the in-memory rollback couldn't take back the disk line the flush wrote, so a restart hid a chip for a DELETE that failed. Fix: the requesting slot, its aliases, and any late-joiner now carry a _dismissed_txn_in_flight flag set under _source_link_txn_lock around the mutate→persist→rollback; while it's set, BOTH full-save paths (full save + empty-window merge save) carry the on-disk dismissed line forward instead of serializing the tentative set — exactly as they already do for an unhydrated slot. The flag clears on commit (authoritative set already on disk) and on rollback. Two regressions added (handler: flag True during the write + cleared on failed persist; persistence: an in-flight full save carries the on-disk line, drops the tentative key). 263 backend tests + both baseline gates (drift-free) + black/flake8/mypy green.

[RohanK6]

This is ready for review after resolving merge conflicts

[RohanK6]

This is ready for review – all merge conflicts are resolved