feat(chat): zero-turn option actions + visible-only note mode - #6823
feat(chat): zero-turn option actions + visible-only note mode#6823rnoack1 wants to merge 1 commit into
Conversation
55af774 to
20fbfe8
Compare
UX Review (Fable 5, fork) — ⏭️ skippedRevision |
Design Review (Fable 5, fork) — 🟡 CONCERNSDesign-level review of 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
Suggestions
[DESIGN-REVIEWED] 5a6e39b |
GPT 5.6 Review (fork) — 🔴 changes requested (blocking)Reviewed 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 Adjudication (Opus 4.8) — is blocking on each finding proportionate?The adjudicable block is empty (0 findings). One fenced finding, F1. F1 — |
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of I have enough evidence across the patch, the base tree, and the intent file. Every probed claim checked out: 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 shipsIntent: let a user dismiss a session from a model-offered "I'm done" chip without paying an LLM turn — an ADDITION.
Watch
[FIRST-PRINCIPLES-REVIEWED] 5a6e39b |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed Review detailsI've independently reviewed the two highest-risk files — the marker-grammar core in No findings. [OPUS-REVIEWED] 5a6e39b |
20fbfe8 to
3880319
Compare
3880319 to
0745bf5
Compare
0745bf5 to
bc20046
Compare
61d163e to
4d35e95
Compare
4d35e95 to
150f05e
Compare
Open PR relationship auditThis 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
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
|
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: 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 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 ( They compose, and the composition is better than either. #9341's four-alternative body with its And one bonus specifically from your side: your 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 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. |
|
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 ( |
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.
|
@rnoack1 The 2026-09-08 relationship audit read this PR's full 122-file diff (audited at Settle #7157 first. It performs the byte-identical Two premise questions for a human, not a merge. #6831 mirrors Textual conflicts that only need an agreed land order: #8613 (an escalation early return inside the same #7161 collided on the one rewritten line of Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong. |
Problem / Motivation
Clicking an
[OPTIONS:]button always costs a full LLM turn. The label is insertedverbatim 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:
interceptSlashCommandhandles/onboardingas a local action and returns without sending, and option picks alreadyflow through the
send()that runs it. What is missing is a way for an option todeclare 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()appendsthe user row ~274 lines after the interception returns, so
/sideand/onboardingleave no transcript record at all. A close action built that waywould 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
visibleOnlymode has noother 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.confirmCloseSessionis OFF — the session menu, the sidebar✕Alt+Shift+W, and the sidebar's BULK cleanup/Archive, which readsslotHasUnsentWorkper candidate and confirms withcleanup_unsent_confirmnaming the affected sessions. Thetab-strip
✕is NOT one of them and deliberately stays confirm-free:closeSessionTabonly 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.
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.
[OPTIONS:]head remains case-SENSITIVE on the backend while the action head isnot — one counted, deliberately unfixed sibling, deferred on blast-radius grounds. It is
recorded here and not only in a
constants.pycomment, because a deferral that lives in acode comment has no owner. This needs an owner before the casing divergence is treated
as settled.
[OPTIONS:]parsing changes affect traffic models already emit. A same-line siblingmarker 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 secondmarker into the first's final choice label and leaked
[OPTION-ACTIONS: ...]verbatim intothe live Slack bubble and into speech. Each of the four is recorded in
DELIBERATE_CHANGESwith that reasoning.
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:
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.
UNRECOVERABLE_CLAIM_TTL_MS(12h) after a crash, re-confirming that slot's closewith 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.
claimFailureKey/failureIsLive/storageAcceptsClaimsleavesclaimWriteFailed,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
allchunk 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 mainwith ~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.mjsat 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 theclose path, and a
CleanupGuardthe bulk archive holds across its request. It is a tierABOVE 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.
re-reads
slotUnsentWorkSourcebefore deleting, so what the handshake adds is a windowthat 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.
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:
ONE
<action>=<label>entry, split on the FIRST=so a label may contain one; a|is label text, not a separator.closeis the only action; anything unrecognisedis 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 (
onFollowUpSelectandonFollowUpSend) rejecta 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) takingnamed 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 atitle: a disabled button takes no focusand 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
injectrow: writing it before the user could cancel leftthe transcript asserting a close that never happened. The write moved into the close's
own pre-delete window, so the
appended === truegate still governs whether the deleteproceeds.
The dispatch is one shared hook (
useOptionActionDispatch), shaped after therepo's existing
usePlanActionMutation. It was hand-mirrored acrossChatPageandChatPaneWITHIN THIS BRANCH, and those two copies drifted before either wasreviewed — one host's settle-time composer recheck counted two categories of staged
work where the other counted five. Nothing drifted in the product:
useOptionActionDispatchand
hasUnsentComposerWorkhave zero hits in the base tree. It is offered as evidenceabout 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.
visibleOnlyonPOST /api/chat/slots/{slot}/notewrites the visible row andbuilds no context entry. The context half cannot work for a closing tab:
_pending_contextis a plain in-memory list nothing serializes, the close pops theslot, 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', notrole: 'user'(a user row with no assistant reply reads as adropped turn), and carries identifying
meta, because history persistsclsonlyfor
role === "system".The close is sequenced on
appended === true. MEASURED: a note deferred behind arunning turn is held in memory and is not durably recorded at the moment the close is
decided. The close answers
200either way, so there is nothing to catch.The loss that used to follow is FIXED here.
flush_deferred_notes' own docstringclaimed 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_slotnow flushes held notes before the archive save, inside that save'stryso a flush failure shares its restore arm — covering both of its callers, the tab✕ and session control's
close_target. Pinned bytest_a_deferred_note_now_SURVIVES_its_slot_closing, and by a fifth-seam pair inTestSeam5CloseSlotthat 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-ACTIONSover the base is 0 hits; the teachingis 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_TAILisshared 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 patternrequires 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 andtest-pinned, and are called out here because they change behaviour for markers that
predate this PR:
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.
[OPTIONS:]line no longer cancels the marker head. The scannerabandoned 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 nestingdepth 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.
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 thanmangling it. So a frontend-only marker does not no-op, it leaks. Fixed at each site:
has_optionsfalse →waiting_for_inputwith choices on screenChannels 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 markercaptured
['a', 'b]\n[OPTION-ACTIONS: close=…'], which would have shipped the actionmarker 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_*carryIGNORECASEto match the frontend'siflag, so[Option-Actions: close=B]is alive 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 contentbody consumes straight through the sibling, and the captured label becomes
" A] [Option-Actions: close=B"— the action marker handed to every client as auser-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 thatone alternation branch. Widening the content head would change how every pre-existing
[OPTIONS:]marker on every streamed channel message parses, so a test asserts amixed-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, andcomposerHasUnsentWorkflips 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
titleand ansr-onlynode, andaria-describedbyresolves to that node, so a screen reader stillhears all of it.
line-clamp-2bounds the visible height where a translation runslong. New copy landed in all 13 catalogs;
en-XAwas regenerated, not hand-edited.An upload in flight is unsent work.
pendingFilesis written by the uploadRESULT, 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.isPendinginChatPane,uploadinginChatPage(which covers the screenshot/snip path too) — and neither reached
ComposerWork. It isnow 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, sothe chip that deletes the session looked like the one that means "you already picked
this". The same product already paints the same operation
text-dangerinSessionActionsMenu. Tokens verified to resolve in every theme.The audit row is a sentence, not a bare label. The breadcrumb wrote
action.labelalone into a permanent row that renders verbatim in the transcript, soreopening 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}}acrossall 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.
parseActionEntriesyields ATMOST ONE entry and
visibleActionsis deleted: over a one-member enum the dedupe couldnever fire, and the published app-kit protocol already documents
[OPTION-ACTIONS: close=<label>]singular, so the multi-entry capability wasgenerality no caller could reach and no contract promised.
closeno longer returnsPromise<boolean>— zero readers, counted: the menuvoids it, the shared dispatcherdiscards the await, and
ChatSidebar's prop is declared(key: string) => void;beforeDeletestays, which is how a caller still aborts. AndChatInput'scomposerHasDraftnow consumes a sharedhasComposerTextOrFilesrather thanre-spelling those two terms — the text/files half only, deliberately, since
composerHasDraftdrives the mic's hold-to-talk mode and pulling inknowledgeoruploadingthere 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 thestaleness 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 13catalogs; the old
session_closedkey is REMOVED rather than reworded in place, since akey 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.
actionChipTitlereturned the bareaction.label,the label is model-authored free text, and
confirmCloseSessiondefaults tofalse—so
[OPTION-ACTIONS: close=That's all]rendered as✕ That's alland tore the sessiondown on one click with no stated consequence and no dialog. The title is now
Closes this session — {{label}}, and the same string is thearia-label, so namingthe 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.
closetakesforceConfirm, set onlyon 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
knowledgeanduploading, and the worst of them, because the window is WIDEST whenthe composer looks emptiest: a streaming capture that has produced no partial yet
leaves
textempty, so every text-derived term reads false while the user ismid-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
ChatInputand the settle-time recheck the hosts feed. It uses the UNGATEDcapture flag deliberately —
useVoiceInputassignssessionOwneronly after theserver handshake resolves, so the ownership-gated
voiceRecordingreads false whilereal audio is already buffering, which is exactly the cold window.
ChatPaneclaimsvoiceCapture: falsebecause it has no mic at all (zero voice references, nouseVoiceInputsession), 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 watchitem 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_replyfrom its own permissive regex to the sharedANCHORED pattern stopped TTS stripping a mid-prose marker, so
"See [OPTIONS: A | B] for details"— spoken as"See for details"before — beganbeing 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_REfor the removal surfaces, reusing the tempered body so abracket 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_replycarried its ownlocal regex and so spoke the new head aloud.
constants.pyis now pinned as the onlynon-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.pyproducedassert ['voice_reply.py'] == []. Scoped to this marker family and comment-stripped —\[STEERINGis defined locally in three renderers already (a different marker withits own history), and the comment recording that
voice_reply's copy was removedmust 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), whichtook 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 diffrather 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.
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 samebase, so it is not this PR's. Frontend 787 pass across 36 files covering the
changed component, the marker protocol and i18n.
tscintroduces zero errors (22, identical to an unmodified baseline on the same
dependency tree).
eslintclean at the repo ceiling, and all 19 i18n checks pass.New coverage maps to the acceptance criteria:
visibleOnlyand the picked label, before the closeappended: false→ the close is not dispatched;appended: true→ dispatched oncestaged 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
and the slot survives; accepting still writes it before the close
source, with a positive control so a stale extractor cannot pass vacuously
channel block does not, and the runtime selector delivers the teaching to a
dashboard session while withholding it from a channel session
OPTION_ACTIONS_RE_LINEand must yield aknown action with a non-empty label — a prompt teaching an unparseable layout fails
here rather than shipping a producer in name only
exact captured string across four casings and at the
extract_options/_parse_optionslayer a client actually reads — paired with a control proving thecontent head was not widened
sr-onlyone, with a control requiring the two nodes to differ so a truncated-away
explanation cannot pass
constants.pyis the only non-test backend source defining an OPTIONS-familypattern, with a positive control and a stale-exemption check
[OPTIONS:]/[OPTION:]behaviour unchanged, including a turn whoseprose merely discusses the marker syntax
visibleOnlyenqueues no context; defers correctly; rejects a non-booleanThe deferred-close data loss carries a
TestDeferredNoteLostOnClosepair: the defectplus 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 thetwo multi-select suites — could not run in my environment:
tailwindcss-animateisabsent 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.closeinstead of three hand-rolled spellings: the sessionmenu, the sidebar
✕, and theAlt+Shift+Wshortcut inuseKeyboardShortcuts.ts.ChatPage.closeSessionTab(the tab strip✕) is deliberately NOT among them — it onlyrewrites 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 chippasses
forceConfirm, so the model-authored close ALWAYS confirms and names the label theuser clicked. On the product's own routes the user's
confirmCloseSessionpreferencegoverns, and the unsent-draft check only ENRICHES a dialog that was already going to
appear, AND summons one where none was due.
CloseUnsentWorkGuard.test.tsxpins 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.
confirmCloseSessiondefaults 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 stillclosed 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:
chatDrafts,chatFileDrafts,chatPasteDrafts,chatSessionRefDrafts, andsideComposerDraftsfor 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.
localStorage(slotDirtyBeacon.ts,mc-slot-dirty:<composerId>— one key per composer, so no window can overwriteanother'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 yetflushed when another window asks.
localStorage(slotClosingIntent.ts, key familiesmc-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 paysthe 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_MSand is ignored pastCLAIM_TTL_MS— except where the work itnames can be recovered from no store at all, which earns the longer
UNRECOVERABLE_CLAIM_TTL_MSinstead. A browser FREEZES a background tab's timers, so thatwindow 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
beforeunloadretraction, and without abound 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_MSrather than indefinitely. That boundis 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 callsites in
website/src; the three besides this one that a user can reach —useSessionActions.tsvia the session menu,useKeyboardShortcuts.ts, andChatPage.tsx— still delete a session with no record at all. The remainder areprogrammatic 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 theiflag while the backend's CONTENT head is case-sensitive\[OPTIONS:, so[OPTION: A | B]and lowercase[options:already render as pillswhile 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 onappended === true.Verified at source, and the atomic shape is deferred rather than rejected:
close_slotnow flushes heldnotes, so a close no longer discards one. An earlier revision of this description
said the opposite; that claim is withdrawn.
breadcrumb WRITE into
close_slotwould change the close path for a second callerthis PR does not otherwise touch, because
close_slotis SHARED withsession-control's
close_target.has landed, which a single atomic call cannot express.
So the two-phase sequence stays for this PR, and the
appended === truegate is whatmakes 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-casingdivergence 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.