Skip to content

feat(chat): zero-turn option actions + visible-only note mode - #6823

Open
rnoack1 wants to merge 1 commit into
kirodotdev:mainfrom
rnoack1:feat/zero-turn-option-actions
Open

feat(chat): zero-turn option actions + visible-only note mode#6823
rnoack1 wants to merge 1 commit into
kirodotdev:mainfrom
rnoack1:feat/zero-turn-option-actions

Conversation

@rnoack1

@rnoack1 rnoack1 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

Clicking an [OPTIONS:] button always costs a full LLM turn. The label is inserted
verbatim into the composer and sent as a user message, so even an option whose whole
meaning is a local UI action — "Nothing else, close this tab" — pays a model
round-trip that buys nothing. A purpose-scoped tab (a decision tab) cannot be
dismissed straight from the option the user just picked.

The seam for a zero-turn dispatch already exists: interceptSlashCommand handles
/onboarding as a local action and returns without sending, and option picks already
flow through the send() that runs it. What is missing is a way for an option to
declare an action, and a durable record that the click happened.

Why it matters

The existing zero-turn precedent has a defect this must not copy. send() appends
the user row ~274 lines after the interception returns, so /side and
/onboarding leave no transcript record at all. A close action built that way
would make a tab vanish with nothing in the log naming the click that did it.

That is why this ships the note-endpoint change in the same PR rather than splitting:
the action is not safe without the audit trail, and the visibleOnly mode has no
other caller, so splitting would land dead code.

Declared riders on PRE-EXISTING behaviour — read these as changes in their own right

Stated here, above ## What changed, because a reader who stops early must still meet them:
each alters something that shipped before this PR, and none is mere plumbing for the chip.
The fuller treatment of each is further down under ### Declared rider: and ## Tests.

  1. Four pre-existing close routes now confirm over an unsent draft even when
    confirmCloseSession is OFF
    — the session menu, the sidebar Alt+Shift+W, and the sidebar's BULK cleanup/Archive, which reads slotHasUnsentWork per candidate and confirms with cleanup_unsent_confirm naming the affected sessions. The
    tab-strip is NOT one of them and deliberately stays confirm-free: closeSessionTab
    only rewrites the tab list, so the session and its draft both survive and there is
    nothing to warn about. The preference governs the habitual "are you sure"; silencing it is not
    consent to lose the only copy of a draft. An empty composer still closes in silence. This
    is a default-config behaviour change on controls this PR is not otherwise about, and it
    deserves review as such rather than as a detail of the chip.
  2. A mode switch now CARRIES unsent work to the replacement slot instead of deleting
    it, and session-resume now confirms before retiring the tab it replaces. Both change
    pre-existing flows: the mode toggles previously erased the old slot's draft buckets
    outright, and resume deleted the active tab with whatever was typed in it. Neither is part
    of the chip; both are draft-loss fixes on routes this PR happens to touch.
  3. The [OPTIONS:] head remains case-SENSITIVE on the backend while the action head is
    not
    — one counted, deliberately unfixed sibling, deferred on blast-radius grounds. It is
    recorded here and not only in a constants.py comment, because a deferral that lives in a
    code comment has no owner. This needs an owner before the casing divergence is treated
    as settled.
  4. [OPTIONS:] parsing changes affect traffic models already emit. A same-line sibling
    marker now parses, a nested citation bracket no longer cancels a marker head, and Slack's
    live stream holds bracketed text to end-of-line before deciding. Existing lines can
    therefore be re-graded — a line carrying [1] gains chips where it previously had none.
    The merge-base-captured characterization corpus is what separates "additive" from
    "unnoticed reparse", and it now covers ALL THREE shapes this rider re-grades. The gap this
    description previously declared is CLOSED: the capture run has been done, at a worktree on
    the merge base with this change absent, and the harness was validated first by reproducing
    all 13 pre-existing rows byte-exact before its new rows were trusted. 12 corpus cases, 15
    fixture rows. What it measured is worth stating: the citation shape moves NOTHING, so it is
    provably additive; the same-line-sibling shape moves four columns and every one CORRECTS a
    base defect -- the old body ran greedily to the line's last ], so it swallowed a second
    marker into the first's final choice label and leaked [OPTION-ACTIONS: ...] verbatim into
    the live Slack bubble and into speech. Each of the four is recorded in DELIBERATE_CHANGES
    with that reasoning.
  5. A cross-window dirty-claim beacon (slotDirtyBeacon.ts) is new frontend protocol
    per-composer localStorage claims, two TTLs, refresh stamps, per-composer failure records
    and a memoised writability probe. Its payoff is bounded: it lets a close confirm rather
    than silently delete another window's unsent work. Three dispositions on it are recorded
    explicitly, so they are decisions rather than omissions:
    • Fail-closed is deliberate and its cost is real. With storage blocked or unreadable
      the guard answers "unsent work may exist" for every slot, so a storage-blocked browser
      confirms on every close for as long as storage stays blocked. Kept because the
      alternative is the failure mode this guard exists to stop: reading a clean slot and
      deleting the only copy of a draft. Note what else is true in that state — no draft
      persists either, so the browser is already degraded well beyond this dialog.
    • An unrecoverable-work claim can outlive its work by up to
      UNRECOVERABLE_CLAIM_TTL_MS (12h) after a crash
      , re-confirming that slot's close
      with nothing on screen. Bounded rather than exempt: a browser FREEZES a background
      tab's timers, so a live window holding a pending knowledge selection, an in-flight
      upload or an active voice capture misses the 25s re-stamp while the work is still on
      screen, and expiring on the refresh scale destroyed it. 12h is a floor under "no window
      lives this long silent", not an estimate of how long an upload takes.
    • The failure-record tier is kept, per-composer, rather than deleted. Deleting
      claimFailureKey / failureIsLive / storageAcceptsClaims leaves claimWriteFailed,
      which is module-local — so a window whose claim write failed would again be invisible to
      the healthy window doing the closing. That is the precise defect a prior review round
      graded blocking, so the subtraction would reintroduce it. The record is instead keyed
      per composer, and only the composer that recovers clears its own.

Bundle ceiling: nothing owed here any more

Upstream re-measured the all chunk ceiling on its own (fix(ci): re-measure the drifted all-chunk bundle ceiling), taking it to 10975 KB after measuring the chunk at 10450 KB on main
with ~5% headroom. Its rationale names exactly the failure this branch hit: a ceiling that had
drifted to under 1% headroom fails "on routine string growth rather than on the new library it
exists to catch".

This branch measured its own chunk at 10501 KB, which sits 474 KB inside upstream's ceiling, so
the earlier raise carried here is redundant and has been dropped in the rebase. This PR no
longer touches website/scripts/check-bundle-size.mjs at all.

6. A second cross-window protocol, the closing-intent handshake (slotClosingIntent.ts),
is also new
— and it was missing from this list, which is the omission the Design and
First Principles reviews both named. Declared now in its own right: a nonce'd
intent/veto/presence exchange over localStorage, a 400ms acknowledgement window on the
close path, and a CleanupGuard the bulk archive holds across its request. It is a tier
ABOVE the beacon in rider 5, not part of it, and it changes pre-existing close timing:
every guarded close now waits out that window before deleting.

  • Its marginal coverage is narrow, and stating it is the point. The close path already
    re-reads slotUnsentWorkSource before deleting, so what the handshake adds is a window
    that can ASK another window, plus a durable claim written before any veto. First
    Principles argues that is not worth ~300 lines and proposes deleting the tier; that
    judgement is open and belongs to a human, not to this description.
  • The commit-boundary residual is now closed on the client side. A veto is re-read
    immediately before the DELETE with nothing between, and again once the server has
    committed; a refusal landing during a bulk archive no longer evicts its slot, so the
    composer holding that draft stays mounted. What client code still cannot do is make the
    round-trip atomic: a keystroke that lands after the request is on the wire is reported,
    not prevented, and closing that needs the server to carry a close token.

What changed

A structured action marker. Proposed spelling, and the part most worth arguing:

[OPTION-ACTIONS: close=Nothing else, close this tab]

ONE <action>=<label> entry, split on the FIRST = so a label may contain one; a
| is label text, not a separator. close is the only action; anything unrecognised
is dropped, never dispatched.

The action is deliberately not encoded in the label. Marker content is
model-emitted prose, so a magic label or prefix would mean an agent quoting an
example — or writing docs about this very feature — closes the reader's tab. The
action lives in its own field and the label stays arbitrary free text. The tree
already carries a carve-out for prose that merely discusses the marker syntax,
which is the same hazard one step removed.

The safety property that actually carries this is nothing auto-dispatches: a
spurious marker produces a button, not a close. The user must click. That is a
stronger guarantee than the label rule alone, and it is the one to review.

One click entry point per action chip. The ordinary chip has three — a
single click, a double-click direct send, and the split-button arrow. The existing
plan-action chips intercept only the first, so a double-click still sends their label
as chat text. Action chips take no debounce, no arrow segment and no quick-send
branch, and both host content seams (onFollowUpSelect and onFollowUpSend) reject
a label owned by a live action.

The chip is disabled while the composer holds ANYTHING unsent — a content pick,
typed text, a staged file or directory, a session reference, a collapsed paste block,
or a pending knowledge selection. A close tears the composer down and the composer is
its only copy. The predicate is one shared function (hasUnsentComposerWork) taking
named required fields. It started, IN THIS BRANCH, as positional varargs, and that
shape let a whole category go missing with no type error twice over the course of
drafting it — on the argument list of a destructive gate. Neither draft ever shipped;
the point is that the shape made the omission invisible, which is why the field list is
required rather than optional. The reason a chip is blocked is rendered as visible text and
wired via aria-describedby, not left in a title: a disabled button takes no focus
and gets no hover, so a tooltip reached neither a keyboard nor a touch user.

Order is confirm → breadcrumb → close. The confirm comes first because the
breadcrumb is a permanent inject row: writing it before the user could cancel left
the transcript asserting a close that never happened. The write moved into the close's
own pre-delete window, so the appended === true gate still governs whether the delete
proceeds.

The dispatch is one shared hook (useOptionActionDispatch), shaped after the
repo's existing usePlanActionMutation. It was hand-mirrored across ChatPage and
ChatPane WITHIN THIS BRANCH, and those two copies drifted before either was
reviewed — one host's settle-time composer recheck counted two categories of staged
work where the other counted five. Nothing drifted in the product: useOptionActionDispatch
and hasUnsentComposerWork have zero hits in the base tree. It is offered as evidence
about the SHAPE, not as an incident: two hand-mirrored copies diverged within days of
being written, which is the failure class the single dispatcher removes. Each host is
wiring only now.

visibleOnly on POST /api/chat/slots/{slot}/note writes the visible row and
builds no context entry. The context half cannot work for a closing tab:
_pending_context is a plain in-memory list nothing serializes, the close pops the
slot, and resume rebuilds from the transcript with an empty queue — so the visible
row is what carries meaning into a reopened session. The breadcrumb is
role: 'inject', not role: 'user' (a user row with no assistant reply reads as a
dropped turn), and carries identifying meta, because history persists cls only
for role === "system".

The close is sequenced on appended === true. MEASURED: a note deferred behind a
running turn is held in memory and is not durably recorded at the moment the close is
decided. The close answers 200 either way, so there is nothing to catch.

The loss that used to follow is FIXED here. flush_deferred_notes' own docstring
claimed every withholding seam "is followed by a seam that DOES flush … and the
bulk-cleanup archive for a slot torn down before any of them run"; that enumeration
omitted the single-tab close, which is exactly the path the ✕ button and this action
take. close_slot now flushes held notes before the archive save, inside that save's
try so a flush failure shares its restore arm — covering both of its callers, the tab
✕ and session control's close_target. Pinned by
test_a_deferred_note_now_SURVIVES_its_slot_closing, and by a fifth-seam pair in
TestSeam5CloseSlot that fails without the flush.

The client gate stays regardless: a note the backend has not committed is still not the
record the action exists to leave, so a deferral keeps the tab and says why.

A producer: the dashboard prompt teaches the marker. Without this the pipeline is
unreachable — every other site in this PR parses, strips or renders a marker that
nothing would ever emit, so the named harm (a full model call spent answering "close
this tab") would survive the merge untouched while models kept emitting a plain
[OPTIONS:] entry. grep OPTION-ACTIONS over the base is 0 hits; the teaching
is now the one producer site among 21 hit-bearing files, the rest being
parse/strip/render, test and docs.

It is dashboard-only by construction, not by wording. _CRITICAL_RULES_TAIL is
shared verbatim by the dashboard and channel blocks — only the diff rule differs —
so the teaching is a separate segment concatenated into the dashboard block alone.
Putting it in the shared tail would spend tokens on every channel turn for an
instruction whose only possible outcome is a dropped marker, since every channel
renderer here strips it.

The taught example is verified against the shipped extractor, not string-matched.
[OPTION-ACTIONS: close=…] anything else? parses to nothing — the pattern
requires the marker to end its line — so a prompt teaching the field syntax without
the placement would have produced markers that silently never fire, passing every
other test in this PR. Measured across six layouts: both markers sharing one final
line parse in either order, which is what lets a close chip sit beside ordinary
choices without breaking the "[OPTIONS:] is the final line" rule.

Two declared riders on existing [OPTIONS:] grammar. Both are deliberate and
test-pinned, and are called out here because they change behaviour for markers that
predate this PR:

  1. Slack's live stream now holds bracketed text to end-of-line before deciding.
    The suppressor used to drop a partial marker the moment it saw [, so a ]
    inside a label left the tail leaking into the live bubble. It now settles at the
    end-of-stream flush by excising each complete marker span and releasing the
    remainder, so ordinary bracketed prose survives rather than being swallowed.
  2. A bracket nested inside an [OPTIONS:] line no longer cancels the marker head. The scanner
    abandoned a head on any further [, so a line carrying a citation-style bracket —
    [OPTIONS: see [1] | other] — rendered no chips and leaked raw. It now tracks bracket nesting
    depth AND kind, so a closer pops whichever bracket is innermost and the head survives an inner
    bracket. A pre-existing line of that shape gains chips where it previously had none.
  3. Two markers on ONE line now both parse. The line tail terminates before a
    same-line sibling marker; previously the leading marker's body swallowed the
    second and the pair leaked raw. The producer above depends on this — it is what
    makes a close chip offerable on the same final line as ordinary choices.

Marker leaks on non-dashboard surfaces. Every existing parser keys on the
literal [OPTIONS: and, on a non-match, passes text through verbatim rather than
mangling it. So a frontend-only marker does not no-op, it leaks. Fixed at each site:

Surface Was
Slack (buffered) raw marker posted to the channel
Slack (streaming) raw marker in the live bubble
TTS / voice marker read aloud (own hand-rolled regex)
Sidebar preview marker in the one-line preview
Session status has_options false → waiting_for_input with choices on screen

Channels strip and drop rather than rendering a button: a Slack button cannot
close a dashboard tab.

Both tempered bodies now exclude every marker head, from one shared definition. A
body tempering only against its own head still matches across the other one —
measured on the TRAILER form, where [OPTIONS: a | b] followed by an action marker
captured ['a', 'b]\n[OPTION-ACTIONS: close=…'], which would have shipped the action
marker as a channel button label. Additive: text lacking the other head matches
byte-identically.

The shared head alternation is case-aware PER HEAD. OPTION_ACTIONS_RE_* carry
IGNORECASE to match the frontend's i flag, so [Option-Actions: close=B] is a
live marker the dashboard renders a chip for — but the temper and the same-line
sibling lookahead spelled the head case-sensitively, so the content pattern did
not recognise that same text as a head. Measured on
[OPTIONS: A] [Option-Actions: close=B]: the negative lookahead succeeds, the content
body consumes straight through the sibling, and the captured label becomes
" A] [Option-Actions: close=B" — the action marker handed to every client as a
user-visible option label, which is fed back into the session verbatim when
clicked.

Only the action head becomes case-insensitive, through a scoped (?i:…) on that
one alternation branch. Widening the content head would change how every pre-existing
[OPTIONS:] marker on every streamed channel message parses, so a test asserts a
mixed-case content sibling still behaves exactly as before — that control is what
makes the fix per-head rather than a blanket flag.

The blocked-chip reason is one visible line. The full ~90-char consequence at
text-[11px] in a narrow column wrapped to 4-6 lines, and composerHasUnsentWork
flips true on the first typed character — so on any row offering a close chip, the
bar directly above the caret grew by several lines the moment the user began a reply
and collapsed again on send. Churn on every draft. The explanation is not truncated
away: the short form is visible, the full sentence remains the title and an
sr-only node, and aria-describedby resolves to that node, so a screen reader still
hears all of it. line-clamp-2 bounds the visible height where a translation runs
long. New copy landed in all 13 catalogs; en-XA was regenerated, not hand-edited.

An upload in flight is unsent work. pendingFiles is written by the upload
RESULT, not by the file picker, so between the picker closing and the response landing
every other term of the guard reads false while the user has already committed an
attachment. A close in that window deleted the pane, the upload resolved into a slot
that no longer existed, and the file was gone with no error raised. Both hosts already
had the state — uploadMutation.isPending in ChatPane, uploading in ChatPage
(which covers the screenshot/snip path too) — and neither reached ComposerWork. It is
now a REQUIRED field, so a host cannot omit it silently, and it is threaded at the
render gate as well as the settle-time recheck.

The destructive chip wears the danger palette. Its accent paint was byte-identical
to chipColors(true) — a selected content chip — apart from the background tint, so
the chip that deletes the session looked like the one that means "you already picked
this". The same product already paints the same operation text-danger in
SessionActionsMenu. Tokens verified to resolve in every theme.

The audit row is a sentence, not a bare label. The breadcrumb wrote
action.label alone into a permanent row that renders verbatim in the transcript, so
reopening from history showed "Nothing else, close this session" with no actor and no
outcome — it read as something the user had said, which is the opposite of the audit
job the row exists for. It is now a catalog-keyed Session closed — {{label}} across
all 13 catalogs. The failed-write notice likewise names the outcome and the next step
instead of a bare "Connection error", since the distinction that matters to the user is
that the session is still open.

Three subtractions, each counted at source first. parseActionEntries yields AT
MOST ONE entry and visibleActions is deleted: over a one-member enum the dedupe could
never fire, and the published app-kit protocol already documents
[OPTION-ACTIONS: close=<label>] singular, so the multi-entry capability was
generality no caller could reach and no contract promised. close no longer returns
Promise<boolean> — zero readers, counted: the menu voids it, the shared dispatcher
discards the await, and ChatSidebar's prop is declared (key: string) => void;
beforeDelete stays, which is how a caller still aborts. And ChatInput's
composerHasDraft now consumes a shared hasComposerTextOrFiles rather than
re-spelling those two terms — the text/files half only, deliberately, since
composerHasDraft drives the mic's hold-to-talk mode and pulling in knowledge or
uploading there would silently change voice behaviour for a refs-only composer.

The audit row records the REQUEST, not the outcome. It said Session closed — {{label}}, and that could be a permanent false statement. The write has to precede the
staleness and composer rechecks, because those exist to catch state that moved DURING
the POST — a draft typed inside that window is invisible to the render-time gate — so a
recheck can abort the close when the row is already durable. The user then saw
"Session closed", an error row, and a tab still sitting there, and reopening from
history showed that forever. It is now Close requested — {{label}} in all 13
catalogs; the old session_closed key is REMOVED rather than reworded in place, since a
key by that name holding "Close requested" is the next reader's trap.

Writing the row only after every gate cleared was the other candidate and is strictly
worse: it puts both rechecks before the only await, which is exactly the blind spot
they were added for, trading a mis-worded row for closing over the user's unsent draft.
The row states what is actually guaranteed at write time.

The chip now says what it does. actionChipTitle returned the bare action.label,
the label is model-authored free text, and confirmCloseSession defaults to false
so [OPTION-ACTIONS: close=That's all] rendered as ✕ That's all and tore the session
down on one click with no stated consequence and no dialog. The title is now
Closes this session — {{label}}, and the same string is the aria-label, so naming
the effect is the chip's accessible NAME rather than a hover-only extra. Deliberately
NOT the block reason when disabled: a button's name must identify the button, and the
reason is already announced as the DESCRIPTION via aria-describedby.

The marker-originated close always confirms. close takes forceConfirm, set only
on this path, because the affordance was authored by a model rather than by the product.
A caller that put the affordance there itself — the session menu, a keyboard shortcut —
leaves it unset and the user's preference still governs.

The abort notice has its own string. It reused the chip's helper copy, which
explains why a chip is disabled; this is an error about a close that did not happen,
and what the user needs told is that the session stayed open.

An active voice capture is unsent work. The third instance of the same hole as
knowledge and uploading, and the worst of them, because the window is WIDEST when
the composer looks emptiest: a streaming capture that has produced no partial yet
leaves text empty, so every text-derived term reads false while the user is
mid-sentence. Clicking the chip there closed the slot, which disarmed voice, and the
final transcript was dropped with nothing on screen to show it had existed.

Threaded through both composer-work checks, since one is not a fix: the render-time
gate in ChatInput and the settle-time recheck the hosts feed. It uses the UNGATED
capture flag deliberately — useVoiceInput assigns sessionOwner only after the
server handshake resolves, so the ownership-gated voiceRecording reads false while
real audio is already buffering, which is exactly the cold window. ChatPane claims
voiceCapture: false because it has no mic at all (zero voice references, no
useVoiceInput session), which is the point of the required fields — it is a claim,
not an omission.

A regression on pre-existing [OPTIONS:] traffic, found and fixed. Design's watch
item was that this PR rewrites machinery the OLD marker shares, and that such a
regression would read as unrelated inside a 50-file PR. It was right, and it had
already happened: repointing voice_reply from its own permissive regex to the shared
ANCHORED pattern stopped TTS stripping a mid-prose marker, so
"See [OPTIONS: A | B] for details" — spoken as "See for details" before — began
being read aloud in full, the worst surface for the artefact to reach.

The cause was conflating two different questions. A dispatch parser must NOT treat
mid-prose brackets as a marker (that carve-out is deliberate — a sentence discussing
the syntax must not become a button); a stripping surface must delete them precisely
BECAUSE they are prose the user never meant to hear. So there is now a
MARKER_STRIP_ANYWHERE_RE for the removal surfaces, reusing the tempered body so a
bracket inside a label still does not leave a tail.

Pinned by a CHARACTERIZATION test whose expectations were captured from the pre-PR
implementation
on a pristine worktree at the merge-base, not authored: 12 shapes must
match byte-for-byte, and the 4 remaining changes are asserted explicitly as intended
(the declared same-line rider, the nested-bracket head rider, the bracket-in-label tail fix, and CJK closer support).
A fourth change to pre-existing behaviour cannot now appear unnoticed.

A backend marker-definition ratchet. The frontend already asserts the protocol
module is the only non-test source that defines the markers; the backend had no such
rule, which is exactly where this PR's leak came from — voice_reply carried its own
local regex and so spoke the new head aloud. constants.py is now pinned as the only
non-test backend source defining an OPTIONS-family pattern, with a positive control so
the scan cannot pass vacuously and a stale-entry check so an exemption cannot outlive
its reason. Proven able to fail: planting a pattern in voice_reply.py produced
assert ['voice_reply.py'] == []. Scoped to this marker family and comment-stripped —
\[STEERING is defined locally in three renderers already (a different marker with
its own history), and the comment recording that voice_reply's copy was removed
must not count as a definition.

Two grep ratchets were re-pointed, not weakened. Generalising the scan removed
the literals they keyed on (rfind("[OPTIONS") and a tempered-body substring), which
took both vacuous — caught by the anti-vacuous test that exists for exactly that.
Each now anchors on a needle that must exist and pins the property rather than one
brittle substring.

Screenshots

Committed under temp-screenshots/zero-turn-option-actions/, so they travel with the diff
rather than living only on a fork ref. Generated by
website/scripts/capture-zero-turn-option-actions.mjs, which asserts as it photographs:
it fails if fewer than four chips render or if a blocked panel's reason comes out empty.

Panel 1 is the action chip alone (the ✕ glyph, the model-authored label, danger paint).
Panel 2 puts it beside two content chips, showing the divider that keeps a one-click local
action away from the chips that merely send text. Panels 3-6 walk every reason the blocked
state can state: an unsent draft here, a staged pick, a capture in flight, and a draft held
in another pane.

Zero-turn option-action chip states, dark theme

Zero-turn option-action chip states, light theme

Tests

Backend 102 pass in the marker suite alone. Wider sweep over everything touching
marker/options/context/prompt/slack/renderer/voice/telegram/discord/messaging:
8036 pass, 1 fail — that one
(test_design_tweak_backend.py::…::test_prose_quoting_a_pem_marker_still_serves,
assert 403 == 200) reproduces byte-identically on an unmodified worktree at the same
base, so it is not this PR's. Frontend 787 pass across 36 files covering the
changed component, the marker protocol and i18n. tsc
introduces zero errors (22, identical to an unmodified baseline on the same
dependency tree). eslint clean at the repo ceiling, and all 19 i18n checks pass.

New coverage maps to the acceptance criteria:

  • clicking an action chip starts no LLM turn
  • the breadcrumb carries visibleOnly and the picked label, before the close
  • appended: false → the close is not dispatched; appended: true → dispatched once
  • a refused close writes no second breadcrumb and claims nothing
  • an action chip is disabled while content picks exist, and also for typed text, a
    staged file, a collapsed paste block and a pending knowledge selection — the
    last leaves no token, so every text-derived term reads false while the close still
    destroys it
  • with confirm-on-close enabled and the confirm declined, NO breadcrumb is written
    and the slot survives; accepting still writes it before the close
  • the backend action enum is pinned against the frontend's by reading the frontend
    source, with a positive control so a stale extractor cannot pass vacuously
  • a producer exists: the dashboard critical-rules block teaches the marker, the
    channel block does not, and the runtime selector delivers the teaching to a
    dashboard session while withholding it from a channel session
  • the taught example is fed through OPTION_ACTIONS_RE_LINE and must yield a
    known action with a non-empty label — a prompt teaching an unparseable layout fails
    here rather than shipping a producer in name only
  • a mixed-case action sibling no longer corrupts the content label, asserted on the
    exact captured string across four casings and at the extract_options /
    _parse_options layer a client actually reads — paired with a control proving the
    content head was not widened
  • the blocked-chip reason renders a short visible line and a full sr-only
    one, with a control requiring the two nodes to differ so a truncated-away
    explanation cannot pass
  • constants.py is the only non-test backend source defining an OPTIONS-family
    pattern, with a positive control and a stale-exemption check
  • ordinary [OPTIONS:] / [OPTION:] behaviour unchanged, including a turn whose
    prose merely discusses the marker syntax
  • marker non-collision pinned in both directions (the property the design rests on)
  • per-surface strip tests, each paired with an ordinary-marker regression
  • visibleOnly enqueues no context; defers correctly; rejects a non-boolean

The deferred-close data loss carries a TestDeferredNoteLostOnClose pair: the defect
plus a positive control proving an immediate note survives the same close and
reaches disk — without the control, "absent from disk" would be a fact about the
harness rather than the world.

Not verified locally

Three CSS-importing suites — including ChatPage.followUpToggle.test.tsx, one of the
two multi-select suites — could not run in my environment: tailwindcss-animate is
absent from the available dependency tree. They are unmodified by this change and
CI covers them.

Declared rider: the close routes share one confirm, and the draft warning enriches it

This began as an asymmetry — the marker's close was gated on unsent composer work while the
sibling closes were not — and it is fixed in this pass rather than deferred, because the slot
composer registry this PR already adds makes the question answerable from outside the
component that owns the draft. The three sibling routes that DESTROY a session now funnel
through one useSessionActions.close instead of three hand-rolled spellings: the session
menu, the sidebar , and the Alt+Shift+W shortcut in useKeyboardShortcuts.ts.
ChatPage.closeSessionTab (the tab strip ) is deliberately NOT among them — it only
rewrites the tab list, leaving the session and its draft intact, so routing it through a
destructive-close confirm would warn about a loss that does not happen.

What ships, precisely. mustConfirm = confirmCloseSession || forceConfirm. The chip
passes forceConfirm, so the model-authored close ALWAYS confirms and names the label the
user clicked. On the product's own routes the user's confirmCloseSession preference
governs, and the unsent-draft check only ENRICHES a dialog that was already going to
appear, AND summons one where none was due. CloseUnsentWorkGuard.test.tsx pins that,
including the case that discriminates it: "CONFIRMS on a product route with the preference
OFF when a draft exists", plus the abort and the clean-slot control.

Declared rider: unsent work summons its own confirm, whatever the preference says.
confirmCloseSession defaults OFF, and while the draft check was subordinate to it a menu,
sidebar or shortcut close destroyed an unsent draft — a popout's text, an in-flight upload,
an active voice capture — with no warning at all. That subordination is now removed: the
preference governs the HABITUAL "are you sure", and silencing it is not consent to lose the
only copy of a draft. This DOES change a pre-existing route's behaviour under default
config, which is why it is declared here rather than folded in silently: a user who
switched routine confirms off will now see a dialog on the session menu, the sidebar
and Alt+Shift+W, but ONLY when a draft would be lost. An empty composer is still
closed in silence, and the late re-check now also runs on every route, because work typed
while the close awaited the network is precisely the case a preference about routine
confirms cannot speak to. An earlier revision of this branch reverted this, on the reasoning
that forcing a dialog overrode a stated preference on routes the PR was not otherwise
changing; that reasoning treated the preference as covering draft loss, which it does not.
Where a dialog appears, the prompt names
the draft loss rather than asking a generic "are you sure", and it is APPENDED to any
caller-supplied message so the marker close still names the label clicked.

What the close guard actually consults, and what is left. The registry answers only
for MOUNTED composers — an entry is added on mount and removed on unmount — so a registry-only
guard finds nothing for a slot whose pane is not mounted. This PR therefore does NOT stop at the
registry; the earlier draft of this section said lifting drafts into the store "remains a change
of its own", and that is no longer true of the shipped diff. Four layers now answer:

  1. The REGISTRY, for composers mounted in this window.
  2. The PERSISTED drafts (chatDrafts, chatFileDrafts, chatPasteDrafts,
    chatSessionRefDrafts, and sideComposerDrafts for the side and embedded composers,
    which held their text in React state alone). These never expire, so they are what makes
    an unmounted or frozen surface's draft discoverable at all.
  3. A cross-window claim in localStorage (slotDirtyBeacon.ts,
    mc-slot-dirty:<composerId> — one key per composer, so no window can overwrite
    another's), published synchronously on each dirty transition. It exists because the persisted write is debounced
    by up to DRAFT_SAVE_DEBOUNCE_MS, so a draft being actively typed in a popout has not yet
    flushed when another window asks.
  4. A closing HANDSHAKE in localStorage (slotClosingIntent.ts, key families
    mc-slot-closing:intent:<slot>, :veto:<nonce>:<composerId> and :present:<composerId>).
    Before a close deletes, it announces a nonce'd intent and waits CLOSING_ACK_WINDOW_MS
    (400 ms) for a live composer in another window to flush its claim and veto. Its limit is
    stated rather than implied: layer 3 publishes in a layout effect, so this NARROWS the gap
    between this thread's last tier read and the DELETE without closing it — a keystroke can
    still land inside the round trip. What it adds over a post-ack beacon re-read is an
    ANSWER from the holding composer (a synchronous flush plus an explicit refusal) rather
    than another poll of the same debounced surface.

Its cost, also stated: presence is deliberately NOT slot-keyed, because a composer's slot is
dynamic (getSlot()) and a stamped slot would go stale the moment it moved — so a close pays
the 400 ms window whenever ANY other window holds ANY composer, clean or not. A lone window
pays nothing: with no foreign presence key the intent is never published and the close stays
synchronous.

The beacon's cost, stated rather than hidden: a claim carries a timestamp its owner refreshes
every SLOT_DIRTY_REFRESH_MS and is ignored past CLAIM_TTL_MS — except where the work it
names can be recovered from no store at all, which earns the longer
UNRECOVERABLE_CLAIM_TTL_MS instead. A browser FREEZES a background tab's timers, so that
window misses its re-stamp while still holding the work, and the refresh-scale TTL would hand
another window a clean slot to delete. Bounded rather than exempt: a window that CRASHED
mid-upload must eventually let the slot go, since the work went with it. Expiry is deliberate — a
window that CRASHES runs neither its unmount nor its beforeunload retraction, and without a
bound its orphaned claim would refuse that slot's close forever with no way for the user to
find or clear the draft it names. The trade is that a claim can outlive its draft by up to the
TTL, so a close refused in that window is retryable; and because a browser cannot distinguish a
crashed window from a merely FROZEN one, expiry is only safe where the draft is persisted —
which is why layer 2 now covers the side and embedded composers. Timer throttling in a hidden
tab is bounded by the same margin between refresh and TTL.

The guard FAILS CLOSED when storage will not hold a claim. A claim that never reached
storage is invisible to every other window, so an absence of claims under a refusing store
reports dirty rather than clean; quota is per-origin, which is what makes a write THIS
window cannot land evidence about the others. Over-reporting costs a confirm the user can
dismiss, under-reporting costs the only copy of their text.

What is genuinely left: work that persists NOWHERE — a pending knowledge selection, an upload
in flight, an active voice capture — is visible only through the registry and the beacon, so it
is protected for as long as UNRECOVERABLE_CLAIM_TTL_MS rather than indefinitely. That bound
is a floor under "no window lives this long silent", not a guess at how long an upload takes,
and it is the honest limit of a claim standing in for work nothing wrote down. Making those
durable means giving each a persisted representation, which is a change of its own.

What makes the marker's close the one that also earns an audit row is unchanged, and is the
subject of the next section: PROVENANCE. A model chose to offer it and wrote its label, where
every other path is a user reaching for a control the product placed.

Known asymmetry: the other close paths keep no audit record

This PR argues the marker's close is not safe without an audit trail, and then adds
that trail to ONE of four user-reachable closes. deleteSlot( has 12 non-test call
sites in website/src; the three besides this one that a user can reach —
useSessionActions.ts via the session menu, useKeyboardShortcuts.ts, and
ChatPage.tsx — still delete a session with no record at all. The remainder are
programmatic app cleanup (auto-improvement, issue-radar, design-critique) and a bulk
close in the sidebar.

Stated rather than fixed here, and the asymmetry is real: if the audit trail is what
makes a one-click close acceptable, the menu's has the same problem and no trail.
The difference this PR rests on is PROVENANCE — a model wrote the marker's label and
chose to offer it, where every other path was driven by the user reaching for a control
the product put there. That justifies auditing this one first; it does not justify
leaving the others unaudited forever, and covering them means touching three unrelated
call sites plus deciding what a menu-close row should say, which is its own change.

Deliberately out of scope

A pre-existing divergence worth its own change: the frontend regex is
\[OPTION(S)?: with the i flag while the backend's CONTENT head is case-sensitive
\[OPTIONS:, so [OPTION: A | B] and lowercase [options: already render as pills
while leaking raw into Slack and TTS. Nothing cross-checks the two regexes. This PR
does NOT widen that head — deliberately, since it would re-parse every pre-existing
marker on every streamed channel message, and a test pins that it stays as it is. The
one place the casing had to be reconciled is the shared temper and sibling lookahead,
because the ACTION head is matched case-insensitively there; see the per-head
alternation above.

Design note: why the breadcrumb is client-sequenced

Design review asked for the alternative that was not weighed: an ATOMIC server-side
close, where one call writes the breadcrumb and tears the slot down, instead of the
client sequencing chatSlotNote -> recheck -> delete and gating the close on
appended === true.

Verified at source, and the atomic shape is deferred rather than rejected:

  • The FLUSH half of the gap is fixed here, not deferred: close_slot now flushes held
    notes, so a close no longer discards one. An earlier revision of this description
    said the opposite; that claim is withdrawn.
  • What stays client-sequenced is the ORDER — write, recheck, close. Folding the
    breadcrumb WRITE into close_slot would change the close path for a second caller
    this PR does not otherwise touch, because close_slot is SHARED with
    session-control's close_target.
  • The order is also what buys the abort: a recheck can refuse the close AFTER the write
    has landed, which a single atomic call cannot express.

So the two-phase sequence stays for this PR, and the appended === true gate is what
makes it safe: a note the backend did not durably record does not authorise a close, so
the failure mode is a session left open, never a close with no breadcrumb.

Also still open, and deliberately NOT fixed here: the [OPTIONS:] head-casing
divergence recorded under "Deliberately out of scope" above. Widening that head would
re-parse every pre-existing marker on every streamed channel message, and a test pins
it as it is. It is tracked as a follow-up on its own, not folded into this change.

@rnoack1
rnoack1 requested a review from a team August 29, 2026 18:20
@rnoack1
rnoack1 requested a review from a team as a code owner August 29, 2026 18:20
@rnoack1
rnoack1 requested a review from chenmingwei23 August 29, 2026 18: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 29, 2026
@rnoack1
rnoack1 force-pushed the feat/zero-turn-option-actions branch from 55af774 to 20fbfe8 Compare August 29, 2026 19:59
@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 29, 2026
@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5, fork) — ⏭️ skipped

Revision 8eb2fa7e47445d63474e87e57aa30ed9b8f5d4cc touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.

@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 5a6e39b073e19570e854605abbd62a5b6fb997d3 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

Sound core feature, but it arrives fused to four independently shippable behavior changes and two new client protocols whose keep-or-delete judgment the author explicitly punts to reviewers.

Watch

  • The four-tier draft guard is heavy for its residual: two new localStorage protocols (slotDirtyBeacon.ts, slotClosingIntent.ts, ~560 lines) buy a 400ms wait on every guarded close whenever any other window holds any composer, up-to-12h post-crash confirm dialogs, and confirm-on-every-close under blocked storage — while the description concedes the race is only truly closable server-side ("closing that needs the server to carry a close token") and that a prior review "proposes deleting the tier; that judgement is open and belongs to a human". A human must actually make that call before merge, not after.
  • Riders 1–2 (confirm-over-draft on four pre-existing close routes with the preference OFF; mode-switch draft carry; resume confirm) and rider 4 (re-grading live [OPTIONS:] traffic) are default-config behavior changes on flows the chip does not require. Fused into one 119-file PR, any single regression forces reverting the whole feature; only the visibleOnly endpoint genuinely cannot ship alone.

Suggestions

  • Split into ~3 PRs: marker+chip+visibleOnly+flush fix; the close-route confirm/draft-guard stack; the grammar re-grades — each is independently revertable and matches the repo's one-logical-change rule.
  • Make the server-side close token the tracked follow-up whose landing deletes slotClosingIntent.ts, so the handshake is explicitly interim rather than permanent surface.
  • Don't commit temp-screenshots/ PNGs; PR-hosted images serve the description without permanent repo weight.

[DESIGN-REVIEWED] 5a6e39b

@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

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

1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- website/src/utils/sideComposerDrafts.ts:58 -- The durable side draft never stores its text
const draft: SideDraft = { s: slot, t: Date.now() }
SideChat/ChatEmbed text during close -> presence-only record is marked durable -> composer unmounts and its text cannot be restored.
Anchor: residual/crash-data-loss-corruption
Fix: Persist and restore the text before treating the close-time write as durable.
[BLOCK-MERGE] 5a6e39b
[GPT-REVIEWED] 5a6e39b

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

The adjudicable block is empty (0 findings). One fenced finding, F1.

F1website/src/utils/sideComposerDrafts.ts:58. Confirmed at patch line 17233: writeSideDraft persists { s: slot, t: Date.now() } — presence only, no text. Confirmed at patch line 8867–8871 that useSlotDraftPersistence holds the actual text only in React persisted state during a quiescing close, and the surviving localStorage record carries none of it; on unmount/freeze that in-memory copy dies. The condition (typing into a side/embed composer while its slot closes) is precisely the everyday race the whole handshake/debounce machinery in this PR exists to serve — not an extreme, self-contradicting, or writer-impossible combination. The confirm-dialog mitigation covers the same-window close but not the frozen/crashed-window path the finding names. I cannot complete a rarity record showing the reaching combination is extreme, so a human accepting the residual is not supportable. Harm rung: unbounded (loss of the only copy of user-typed text). When torn on a fenced data-loss finding, the contract requires UPHOLD-FENCED.

[ADJUDICATION] 5a6e39b073e19570e854605abbd62a5b6fb997d3 total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] 5a6e39b073e19570e854605abbd62a5b6fb997d3
[ADJUDICATION-FENCED] 5a6e39b073e19570e854605abbd62a5b6fb997d3 fenced=1 flagged=0
UPHOLD-FENCED F1 website/src/utils/sideComposerDrafts.ts:58 -- The durable record stores slot+timestamp but no text, and the closing window's in-memory copy dies on unmount; the reaching race is the common case the feature targets, not an extreme combination, so residual data loss is not acceptable.
[GPT-ADJUDICATED-FENCED] 5a6e39b073e19570e854605abbd62a5b6fb997d3

@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

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

Premise-level review of 5a6e39b073e19570e854605abbd62a5b6fb997d3 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.

I have enough evidence across the patch, the base tree, and the intent file. Every probed claim checked out: confirmCloseSession defaults false in base (ChatSettings.tsx:61), base OPTIONS_RE_LINE is case-sensitive, temp-screenshots/ is a tracked convention (1198 files + cleanup workflow), the action enum has one variant on both sides with a cross-language parity test, and every new handshake/beacon export has non-test consumers. Final review follows.

First-Principles-Verdict: CONCERNS

Every guard traces to a measured defect, but the counted casing sibling ships owner-less and four pre-existing close routes change default behaviour as riders.

What this change ships

Intent: let a user dismiss a session from a model-offered "I'm done" chip without paying an LLM turn — an ADDITION.

  1. Model can offer a zero-turn close chip — justified (a model round-trip bought nothing)
  2. Chip click leaves a permanent transcript row (visibleOnly note) — justified; 1 consumer, boolean, cause named
  3. Chip close always confirms, naming the model-authored label — justified
  4. Menu/sidebar-✕/shortcut/bulk closes now confirm over drafts even with confirmCloseSession off — declared rider, changed default
  5. Bulk archive skips refusing slots instead of aborting the batch (only_keys) — justified; 1 consumer, narrowing intersection
  6. Mode switch carries drafts; resume confirms; uploads retarget (slotSuccession) — declared rider
  7. Existing [OPTIONS:] lines re-grade (same-line sibling, citation brackets, Slack EOL hold) — declared rider, pinned by merge-base corpus
  8. Two cross-window localStorage protocols: dirty-claim beacon + closing-intent veto handshake — declared riders; races named, all exports consumed
  9. Dashboard system prompt now teaches the marker — justified plumbing for item 1
  10. Backend [OPTIONS:] head stays case-sensitive while the action head is not — declared deferred sibling, count 1

Watch

  • The one unfixed sibling (case-sensitive OPTIONS: head, backend vs frontend i flag — verified in base constants.py:164 vs options.ts) is deferred on blast-radius grounds the diff supports, but the description itself says "this needs an owner" and none is named. Until someone owns it, item 10 is a point patch by the author's own accounting.
  • Item 4 is a shipped-default change on controls this PR is otherwise not about: with the preference off, four routes that closed silently now confirm whenever slotHasUnsentWork answers true — and the beacon fails closed, so a storage-blocked browser confirms on every close indefinitely (cost declared in the description; a human should accept it consciously).
  • Two parallel cross-window protocols is permanent frontend surface; each tier's distinct failure mode (crashed windows vs the ≤300ms debounce race) is documented, so they are not duplicates of each other or of the existing popoutController BroadcastChannel engine — but that is three coordination mechanisms this codebase now carries.

[FIRST-PRINCIPLES-REVIEWED] 5a6e39b

@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

I've independently reviewed the two highest-risk files — the marker-grammar core in constants.py (the tempered bodies, the shared head alternation, the linear _unclosed_marker_flags depth-stack, the strip/match pairing) and the chat_handlers.py close/note-persistence changes. The discovery pass produced zero candidates, so there is nothing to falsify, and my own trace of the load-bearing logic resolves to correct, test-backed behaviour. The one edge worth checking — offset alignment between the ASCII-[ head scan and the opener scan, and the per-line stack reset — is consistent. I have no grounded finding at the required bar.

No findings.

[OPUS-REVIEWED] 5a6e39b

@rnoack1
rnoack1 force-pushed the feat/zero-turn-option-actions branch from 20fbfe8 to 3880319 Compare August 29, 2026 21:02
@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 29, 2026
@rnoack1
rnoack1 force-pushed the feat/zero-turn-option-actions branch from 3880319 to 0745bf5 Compare August 29, 2026 21:58
@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 29, 2026
@rnoack1
rnoack1 force-pushed the feat/zero-turn-option-actions branch from 0745bf5 to bc20046 Compare August 29, 2026 22:10
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 29, 2026
@github-actions github-actions Bot added the readiness: action required A blocking check or review needs attention label Aug 30, 2026
@rnoack1
rnoack1 force-pushed the feat/zero-turn-option-actions branch 6 times, most recently from 61d163e to 4d35e95 Compare August 30, 2026 06:55
@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 30, 2026
@rnoack1
rnoack1 force-pushed the feat/zero-turn-option-actions branch from 4d35e95 to 150f05e Compare August 30, 2026 07:32
@github-actions github-actions Bot removed the readiness: action required A blocking check or review needs attention label Aug 30, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator

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 #6807 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 #6807: MERGE_DISCUSSION. Independent, compatible goals — pre-close gating vs post-close outcome reporting — landing on the same two close call sites, with a verified content conflict in both. The authors should agree an order and a merged shape for close, since the naive resolution silently drops one side's behaviour and breaks 6807's source-text pairing test. Files: website/src/hooks/useSessionActions.ts, website/src/hooks/useKeyboardShortcuts.ts, website/src/hooks/useSessionActions.cov80.test.tsx.
  • PR #6813 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 #6813: KEEP. Independent goals that collide on the same /note handler block and the same api-reference section, and 6813 invalidates the durability premise 6823 records for visibleOnly. Land order and a single reconciled doc paragraph need to be agreed rather than resolved as a mechanical conflict. Files: src/kiro_crew/dashboard/chat_handlers.py, docs/app-kit/api-reference.md.
  • This PR is OVERLAPPING with PR #4904. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6823: MERGE_DISCUSSION. Independent members of one hook; only textual conflict risk. Files: website/src/hooks/useSessionActions.ts.
  • This PR is OVERLAPPING with PR #6831. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6823: MERGE_DISCUSSION. Same endpoint and same docstring region, plus an unconsidered interaction: 6823's breadcrumb becomes channel-visible under 6831. Files: src/kiro_crew/dashboard/chat_handlers.py.
  • This PR is OVERLAPPING with PR #7157. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6823: MERGE_DISCUSSION. Two open PRs widen the same option-protocol interfaces and the same chip component with different, composable features; they need a merge order rather than a choice. Files: website/src/app-sdk/protocol/options.ts, website/src/components/FollowUpBar.tsx, website/src/components/ChatInput.tsx, website/src/components/ChatPane.tsx, website/src/pages/ChatPage.tsx, website/src/pages/chat/SideChat.tsx, website/src/app-sdk/ChatEmbed.tsx.
  • This PR is OVERLAPPING with PR #7163. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6823: MERGE_DISCUSSION. Additive on both sides; worth coordinating the close-time flush's payload semantics, but neither blocks the other. Files: src/kiro_crew/dashboard/slot_buffers.py, src/kiro_crew/dashboard/state.py.
  • This PR is OVERLAPPING with PR #7960. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6823: MERGE_DISCUSSION. Same file regions and the same six consumer surfaces for a parallel marker family; the two should agree on one shared strip pipeline rather than land two independently. Files: src/kiro_crew/constants.py, src/kiro_crew/preview_text.py, src/kiro_crew/voice_reply.py, src/kiro_crew/messaging/renderer.py, website/src/pages/chat/TurnBlock.tsx, website/src/utils/searchableText.ts.
  • PR #7616 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 #7616: KEEP. Complementary changes to the same wiring block; no duplication of behavior. Files: website/src/components/ChatPane.tsx.

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

@cixuuz

cixuuz commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Heads-up on a file collision, and I think a useful one rather than a duplicate.

I opened #9341 (fixes #9284) against the same greedy-body defect your rider #4 describes. It touches five of your files: src/kiro_crew/constants.py, src/kiro_crew/messaging/renderer.py, src/kiro_crew/slack/format.py, src/kiro_crew/whatsapp/turn_renderer.py, website/src/app-sdk/protocol/optionMarker.ts. Measured churn overlap is 88% on constants.py and 139% on optionMarker.ts, so whoever lands second will conflict.

I initially assumed your change already subsumed mine, since your description names the same root cause — "the old body ran greedily to the line's last ]". It doesn't, and the split turns out to be clean. Transcribing all three bodies into a standalone script and running them side by side:

input: 'Use [OPTIONS: Keep it | Drop it] then check arr[0]'
  base    labels=' Keep it | Drop it] then check arr[0'   surviving text='Use '
  #6823   labels=' Keep it | Drop it] then check arr[0'   surviving text='Use '
  #9341   NO MATCH

input: '[OPTIONS: A | B] [OPTION-ACTIONS: close=Close this tab]'
  base    labels=' A | B] [OPTION-ACTIONS: close=Close this tab'
  #6823   labels=' A | B'      surviving='[OPTION-ACTIONS: close=Close this tab]'
  #9341   NO MATCH

We fixed different halves of the same defect:

Your description is explicit that this is deliberate on your side — "Trailing PROSE still does not terminate a marker -- only a sibling marker does" — so I read the gap as scoped-out rather than missed. Worth stating why it matters anyway: every consumer removes the whole match (parseOptions replaces, slack/format.py and messaging/renderer.py cut at match.start(), and whatsapp/turn_renderer.py persists the cut turn), so the swallowed prose is deleted from user-facing text while being simultaneously offered as a button. Under the DOTALL/\Z trailer form the body crosses blank lines, so the whole closing paragraph goes.

They compose, and the composition is better than either. #9341's four-alternative body with its (?!OPTIONS:) widened to your shared _MARKER_HEAD_ALT temper, plus your _MARKER_LINE_END:

PASS  9284 trailing prose    declines
PASS  6823 sibling           labels=' A | B'
PASS  6823 mixed-case        labels=' A | B'
PASS  6823 head-crossing     labels=' a | b'
PASS  ASCII pair in label    labels=' Read arr[0] now | Skip it'
PASS  citation in label      labels=' See [1] above | Skip'
PASS  unmatched closers      labels=' Alpha ] | Bravo ]'
PASS  wrapped marker         labels=' Yes | No'

And one bonus specifically from your side: your MARKER_OPENERS = "[【[〔" is exactly the opener set #9341 says it does not have. #9341 enumerates four accepted costs, and #3 is the lookalike pair[OPTIONS: 见【表1】说明 | 跳过] declines today because is not an opener. Pairing your openers to the closers positionally in the matched-pair branch closes that cost, while still declining a mismatched 【…]:

PASS  CJK pair in label      labels=' 见【表1】说明 | 跳过'
PASS  fullwidth pair         labels=' 见[表1]说明 | 跳过'
PASS  mismatched pair        declines

Caveat on all of the above: I transcribed the patterns from your diff into a standalone script rather than building your branch, and I have not run your test suite. So treat it as a shape check on the grammar, not a verification of your PR.

On sequencing I have no strong claim, and it is the maintainers' call rather than mine. The relevant asymmetry is just that #9341 is 7 files / +846 with 54 tests aimed only at the closer grammar, and this is 123 files / +16205 with a readiness: action required label. If you would rather absorb the closer rule into this PR and have #9341 closed, that is fine by me — say so and I will close it and hand over the tests. If #9341 lands first, the merge into yours is the temper widening plus keeping your _MARKER_LINE_END, and I am happy to do that rebase rather than leave it to you.

One unrelated note on your rider #3, since you asked for an owner: #9341 does not touch head casing, so it neither helps nor blocks that divergence.

@cixuuz

cixuuz commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Follow-up on the openers point above, so it is not left hanging: we are not folding it into #9341. It widens the grammar in the opposite direction from the narrowing that PR is about, and it deserves its own tests and review.

Tracked as #9375 instead, with the measurements and the mismatched-pair caveat ( must not be matched by an ASCII ], or the greedy-body defect returns through a side door). That issue notes your MARKER_OPENERS as the constant it would consume if this PR lands first, and that it introduces the set itself otherwise — so it does not block you either way, and nothing here waits on it.

Adds a second follow-up marker whose click runs a local UI action client-side and
fires no LLM turn. `close` is the only action; unknown actions are dropped.
@bolichen97

Copy link
Copy Markdown
Collaborator

@rnoack1 The 2026-09-08 relationship audit read this PR's full 122-file diff (audited at e5c6b87; head is now 8eb2fa7) and found ten open PRs that meet it at verified same-line sites. Grouped by what each needs:

Settle #7157 first. It performs the byte-identical _CRITICAL_RULES_TAIL / _CRITICAL_RULES_END split in src/kiro_crew/context.py that this PR performs, so a mechanical merge defines that constant twice. It also overlaps seven hunks in website/src/app-sdk/protocol/options.ts, four in website/src/components/FollowUpBar.tsx and three in website/src/components/ChatInput.tsx, and each PR adds a different new field to the same ParsedOptions / deriveFollowUpOptions returns, so a resolution keeping one side silently drops the other's field from some return paths. Land one, rebase the other onto it.

Two premise questions for a human, not a merge. #6831 mirrors /note to the bound channel, which would make the dashboard-only Close requested breadcrumb channel-visible; please decide whether a visibleOnly note is mirrored at all. @CrysisDeu's #8947 gives SideChat a per-slot draft store, which is the gap website/src/utils/sideComposerDrafts.ts was written to fill; ship one side-draft store, not two.

Textual conflicts that only need an agreed land order: #8613 (an escalation early return inside the same deriveFollowUpOptions scan, which would not carry followUpAction), #7163 (_persist_handover_tail), #8852, #6825 and #8599 (all at ChatPane's composer element or the useComposerDraft destructure in SideChat.tsx), and @leonlaiyc's #7616 (same handler block, no line overlap).

#7161 collided on the one rewritten line of website/src/apps/file-explorer/styles.ts; that file is absent from the current head, so that one appears resolved.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

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) merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants