feat(chat): add transcript section markers, gate suggest_followup on human provenance, refuse over-capacity forks - #7163
Conversation
eb21253 to
5af90ad
Compare
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
Design Review (Fable 5, fork) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Well-designed feature, but three independently revertible behavior changes — one a capability withdrawal, one an API-breaking refusal — share one commit's fate. Watch
The marker design itself is sound: RFC-backed with rejected alternatives, positive fail-closed provenance (headless producers refused even with an inherited tab, matching the tool's published contract rather than deleting a pinned decision), reuse of [DESIGN-REVIEWED] 184d3c7 |
UX Review (Fable 5, fork) — 🟡 CONCERNSUX-level review of UX-Verdict: CONCERNS The surface holds up on the diff — but every new pixel (marker row, fork banner) exists only as fork-side binaries no first-time reader has seen. Evidence gaps
Watch
[UX-REVIEWED] 184d3c7 |
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of All evidence is gathered and verified against the base tree. Final review: First-Principles-Verdict: CONCERNS One undeclared rider: Not justified as shipped
What this change shipsIntent: let a reader of a long one-item-at-a-time session see where each unit of work ended — an ADDITION (RFC Phase 1), plus two declared behaviour withdrawals.
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 184d3c7 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed |
5af90ad to
1f961ec
Compare
1f961ec to
8e58499
Compare
8e58499 to
480177e
Compare
iamwhatever
left a comment
There was a problem hiding this comment.
Seems a big design, and also have conflicts with ongoing "crew manager" works which also do summary on session content but showing the "section view" on the right side panels. It is not clear to me how these markers will be commonly available for all providers, not only for kiro-cli.
bolichen97
left a comment
There was a problem hiding this comment.
Description / code mismatch
The Description adds an eighth session directive, but the three places that enumerate DIRECTIVE_TOOLS — including the copy shipped inside the package — still list seven and do not name it.
1. New directive lands as the 8th DIRECTIVE_TOOLS member while three specs and one shipped in-package doc still enumerate seven
The Description says —
An MCP tool and session directive
section_marker(label?, collapse_earlier?), wired the wayreset_conversationis: the stateless tool validates and returns an encoded directive, and the session-aware consumer decodes it at the tool-result event and applies it against its own slot
The code does — section_marker joins DIRECTIVE_TOOLS as its eighth member, and no doc that enumerates that set moves with it:
docs/system-specs/modules/session.md:1030still opens "Seven session-bound MCP tools" and lists seven names.docs/architecture/mcp.md:635-636anddocs/architecture/mcp.md:946-947each enumerate the directive tools withoutsection_marker.src/kiro_crew/docs/messaging-transport.md:123— the copy bundled into the wheel and read by users — carries the same seven-name list.
Risk — AGENTS.md § Specification management requires the spec to move in the SAME commit when an API or documented behaviour changes, and the "Read before you touch" table routes MCP-tool changes to docs/architecture/mcp.md and session/slot changes to docs/system-specs/modules/session.md. The precedent is explicit rather than inferred: commit e2fc1dc0b bumped session.md's count Six→Seven and updated the same enumerations when reset_conversation landed, so the repo maintains these lists as current DIRECTIVE_TOOLS membership, not as frozen history. Docs Lint only checks index links, so nothing reports the drift; the next reader of any of these four lists concludes section_marker is not a session directive. docs/architecture/mcp.md self-describes kiro_crew.mcp_tools.build_tool_list() as the authoritative source, which makes this a stale mirror rather than a contradicted contract — and a stale mirror in a doc that ships to users is still what a reader trusts.
Required change — In this commit, add section_marker to the name list and the count at docs/system-specs/modules/session.md:1030, to both DIRECTIVE_TOOLS enumerations at docs/architecture/mcp.md:634-636 and :945-947, and to the list at src/kiro_crew/docs/messaging-transport.md:123. Naming its dashboard-only, human-provenance, and not-during-stage-execution gates where the other directives' gates are described is desirable but not required — the tool description in control.py and the RFC already carry them.
|
All four enumerations are updated as of The count reads Ten rather than Eight. #5184 added
Line numbers have shifted from the ones you cited, since the branch was rebased a few times in between. |
|
All four enumerations are current as of The count reads Ten rather than Eight because
Line numbers have shifted from the ones you cited, since the branch has been rebased several times since your review. This supersedes my earlier note, which named a head that has since been replaced. |
|
All four required enumerations are updated at head Each site as it reads now:
The line numbers differ from the ones in your review because the base advanced since you wrote it. Content is what I verified, at the head above. One item from your list is still open: naming the dashboard-only, human-provenance, and not-during-stage-execution gates where the other directives' gates are described. You marked that desirable but not required, so the scoping call is yours rather than mine to assume. All three gates do exist in code if you want them documented: Two corrections to the description you quoted, both already applied: it no longer spells the tool |
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. |
|
All four enumerations now name Your line numbers had drifted, so here's what I found against what you quoted:
One correction to the count. Your review said seven going to eight, which was right against Control, since a count claim is easy to get wrong: Your fifth item, the one you marked desirable rather than required, is also in. Head moved from |
|
Re-verified at the current head
On the count, which is the part worth flagging. Rather than carry "Ten" forward from my earlier reply I re-derived it from the registry: Controls, since a count or an absence is a claim about the query rather than about the file. In each of the three files Your fifth item, the one you marked desirable rather than required, is in. One thing I did not change, so it is visible rather than silent: the parenthetical at |
|
All four enumerations name
One correction worth stating, because following your review literally would now introduce a fresh error. You described On completeness, since four cited coordinates are your list rather than proof of the full set. I ran a word-boundary census over every The remaining files that name only a subset are not enumerations of this set. Nothing was changed for this reply. Every site was already correct at this head, so the fix landed in an earlier revision rather than in a new commit. |
|
The required change is in the branch. Verified at head
Your line numbers moved because the spec grew; the content is at the coordinates above. One correction worth flagging, because it changes what the lists say rather than whether they were updated. The review asks for
On the precedent: agreed, and it is the reason the count sentence itself moved rather than just the name lists. The optional item is also done — the three gates specific to this directive (dashboard-only, human provenance, not during multi-stage plan execution) are named in the spec's session-directive section alongside the other directives' gates, plus a deferral subsection covering which flush mode a seam is in. |
|
The required change is in the code. Re-verified at head All four enumerations now list the full set, counted by span:
"Seven session-bound" now appears in 0 files, against a positive control that finds the "Ten One correction to the wording, so the lists stay accurate: the review asks for On the quoted description line — it named On the wiring claim in that same quote, which holds at source on both sides: |
|
Verified at head
Two corrections to the coordinates, so the record is accurate. The lists live further down than One part of the required change was met differently, and I want to be explicit rather than let it I also swept for a fifth enumeration in case one was missed. There is none. Six tracked files On the optional item — naming the dashboard-only, human-provenance and not-during-stage-execution |
|
All four enumerations now name every member, verified at head The three specs and the shipped in-package doc (10 of 10 names each, counted by span rather than per line, since one list wraps):
Two corrections to the premise, both checkable at this head. It is the tenth member, not the eighth: The signature quoted from the description, One part of the required change is deliberately not literal, and it is worth your call. The ask was to add the name "and the count". There is no count at this head: the restated numeral was removed rather than incremented, because a hand-maintained count next to a hand-maintained list is a second thing to forget, and it goes stale silently. What replaces it is a parity test, |
|
All four enumerations you named are addressed at head Two directive tools have landed on the base since your review, so the set is now ten members rather than eight — 1. 2. 3. 4. On your point that nothing reports this drift: |
|
All four enumerations are addressed at head First, the count: two more directive tools landed on the base after your review, so the set is now ten members rather than eight, and 1. 2. 3. 4. I checked whether that enumeration had simply moved to another wheel-bundled doc rather than assuming it was gone. Across all 42 files in On your point that nothing reports this drift: |
|
All four sites you named are addressed at head
On the part of your finding about the copy shipped inside the package — that is now covered by One correction to the framing rather than to the finding: the set holds ten members now, not eight. |
|
All four coordinates you named are addressed at head
One correction to the framing rather than to the finding: the set holds ten members at this head, not seven — Also addressed since your review, in the same spirit: the over-capacity fork refusal no longer restates the row ceiling in localized copy. The server sends the enforced limit and every catalog interpolates it, so the number cannot drift out of step with the constant the backend enforces. |
|
Re-verified at head
Every enumeration of the set now names it:
The "Seven session-bound MCP tool" count word is gone from all of them; grepping the three specs plus the in-package doc for The fourth file you named, |
…human provenance A caller can mark the seam between two units of work; the dashboard draws it as a labelled rule. An in-flight turn defers the append to its boundary (exclude_last_n=1 slices positionally); a multi-stage plan refuses it.
|
@rnoack1 Thanks for this. The audit read the PR at #7779 (@jeeshofone, #8696 (@chenmingwei23) is complementary, since it changes what #9156 (@buluoray) collides on one line in 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
A long-lived chat session is sometimes not one conversation but a sequence of
independent units of work handled one at a time: review this item, finish it,
move to the next. The transcript is a single unbroken scroll, so when unit N+1
starts the reader is looking at the tail of unit N, and nothing in the record
marks where one ended and the next began.
That boundary is information only the writer holds. A client reading the rows
afterwards would have to infer it — from a timestamp gap, a content pattern, a
turn count — and every such heuristic is wrong on the cases that matter (one item
that took four turns; two items handled back to back in one). It is not a
rendering preference a client can compute; it is an assertion about the work.
There is no divider row in the transcript today for any purpose. Every collapse
affordance is intra-turn (
CollapsibleToolGroup,TurnBlock,ToolCallLine),and
EarlierMessagesBaris a server-side pager for history not yet in theclient, not a fold of loaded rows.
Why it matters
Without a way to mark the seam, the only ways to get a clean working surface are
to scroll past everything or open a new session — and a new session per item
loses the sequence, floods the session list, and pays session-start cost per
item.
reset-conversationdoes not help: it deliberately leaves the viewuntouched, and it answers 409 on four guards for a caller that is, by
construction, mid-turn.
This is Phase 1 of the design of record in
docs/request-for-change/rfc-transcript-section-markers.md, which argues theshape and records why four cheaper designs were rejected. Phase 1 ships the
event and the rule and is useful alone: a labelled divider helps in a long
session with no collapse behaviour at all. Collapse is Phase 2 and is
deliberately absent here.
Before you upgrade — two capabilities are WITHDRAWN
Both are on paths unrelated to section markers, and both remove something that worked
before. Neither has a config escape hatch (verified: zero override keys exist).
1. Agent-initiated follow-up cards now require human provenance.
suggest_followupjoins
_USER_ORIGIN_DIRECTIVES(src/kiro_crew/dashboard/session_directive_apply.py:103-107;it is absent from that set on the base). A cron or sub-agent turn riding a slot with an
open dashboard tab could post a follow-up card before this change; after it, that call is
refused. Signal: a WARNING line in the gateway log and a record in the security audit
stream — the card simply never appears, so an automation that posted cards becomes a
silent no-op from the caller's side. Recovery: none in config. A follow-up card has to
originate from a human-driven turn; automations that relied on posting one must deliver
their result another way (
send_message, or a notification).2. Forking an over-capacity conversation now fails instead of truncating. A
whole-conversation fork of a session past
_MAX_SLOT_MESSAGES(10,000 rows) previouslyanswered success while silently dropping the oldest rows; it now answers
400with codefork_corpus_too_large. Signal: the 400 and its code, surfaced in the UI as thefork-failure banner (see Screenshots). Recovery: fork at a message
(
at_message_index/at_message_id) to copy a slice that fits — the refusal messagenames this. The body carries
error,codeandmax_messages— the enforced rowceiling, which the banner interpolates so no localized copy restates it. There is
still no computed retry-at index to read.
Anyone bisecting a broken cron-followup workflow, or an automated whole-fork that used to
return 200, should start here.
Behaviour change beyond the feature
Three independently revertible changes ride this one commit, and that coupling is
a cost rather than a design. They are (1) the section-marker feature itself, (2) the
suggest_followuphuman-provenance withdrawal, and (3) the fork over-capacity refusaltogether with the report argument its refusal copy needs. None depends on another's code, so any
one of them could be reverted alone — but reverting THIS COMMIT reverts all three, which
means a regression in the marker rendering and a rollback of the fork data-loss fix share
one fate. The only shared surface is
_USER_ORIGIN_DIRECTIVES, introduced here andcarrying both
section_markerandsuggest_followup.A directive dropped because its call's identity could not be verified now says so in
the tool result, and that applies to every provenance-gated directive rather than only
to markers. The identity gate refuses a directive whose call does not carry a trusted
_meta.kiroserver name — a forged marker, and equally an ACP backend that emits no suchidentity at all. Before this change that refusal was operator-visible only: a structured
denial row plus a warning naming the observed and expected server, the tool, the session
and the parked-record depth. The agent, meanwhile, saw the tool's own deliberately
non-committal confirmation and had no way to tell a dropped directive from an applied
one, so a backend that cannot carry directives looked identical to one that can. The
result now carries a bracketed
[Not applied: …]note instead, mirroring the note thenative sub-agent refusal already appended. The gate itself is unchanged — silence toward
a genuinely foreign MCP server is the security property, and the note's wording states
only that identity could not be verified.
Every producer of a
suggest_followupdirective was enumerated to confirm no legitimateheadless flow relied on the inherited-tab path. The tool handler
(
mcp_tools/control.py::suggest_followup) is the only emitter; the sole programmaticcall_tool("suggest_followup")in the tree is a test. No bundled skill (0 of 61), app(0 of 496) or task-runner step names it, against a positive control that a sibling
directive
autonudge_stopappears in 4 bundled skills. The tool's published restrictionalready read "cron, and subagent contexts are rejected" before this change, so the
inherited-tab path was an enforcement gap against its own stated contract rather than a
supported route. The agent-facing bullet in the shipped
config/prompt.mdpromised only"Dashboard sessions only", and now states the headless refusal like its three siblings;
a guard reads the gate membership off the applier so a directive added to it later must
say so too.
A fork of an over-capacity conversation now refuses instead of silently losing its oldest
rows, and that is a behaviour change on a path markers need not be involved in at all.
A destination slot trims from the FRONT at
_MAX_SLOT_MESSAGES(10,000), and during the copyloop nothing has reached disk yet, so
persisted_trimis 0 and every evicted row isunrecoverable. Before this change the handler answered success anyway. It now answers
400with code
fork_corpus_too_large, and the error names the alternative that still works:forking at a message (
at_message_index/at_message_id) copies a head or tail slice thatfits. The refusal carries the message, the code and the static
max_messagesceiling: an earlier round also returned a computedat_message_index_that_fits, and that field was CUT before merge because it had a single consumer and would have been a permanent addition to the wire body.test_the_over_capacity_refusal_carries_no_retry_parameterpins its absence, so the 400 body is exactlyerrorpluscode. Markers make this reachable at lower turn counts because they are copied rows too, butany conversation over the ceiling was already affected, including one with zero markers.
Also widened, and outside this feature: a provenance refusal now logs a gateway
WARNING, and it does so for all four provenance-gated directives —
set_project,reset_conversation,section_markerandsuggest_followup— because the log sitsin the shared gate rather than per directive. For the two pre-existing directives,
set_projectandreset_conversation, the SEL audit row was previously the SOLErecord of a refusal; they now emit an operator-visible WARNING as well. Nothing else
about their gating changes.
A provenance refusal now also leaves a line in the gateway log, so an operator whose cron or sub-agent follow-up cards stopped appearing can find out why from the log they actually read rather than only from the security audit stream. It covers ALL FOUR directives behind that gate, which means
set_projectandreset_conversation— outside this feature — also gain a gateway-log line they did not have. That is a deliberate widening: scoping it to the two directives this change gates bought an extra branch and left the other two with the security audit stream as their only record of the same refusal, which is the operator-cannot-see-why gap the line exists to close. Pinned bytest_every_gated_directive_leaves_a_refusal_record.A fork failure now carries a structured report on the notice that already existed, rather than
getting a surface of its own. To be exact about the before-state: fork failures ALREADY rendered
through
ErrorNoticeon the base —ChatPage.tsxrouted bothhandleForkpaths throughshowActionError, and the base carries no livealert()call at all. The gap was the SIGNATURE, notthe slot:
showActionErrortook(message, title?)only, so it had no parameter that could carry thestructured report from the error journal (endpoint, status, backend
code). An earlier round of thisPR answered that with a second, dedicated
fork-errornotice; that was a duplicate surface, since thepage already stacks
upload-error,sid-error,action-errorandpin-errorwith identical styling,dismiss and hand-off behaviour. It is gone.
showActionErrornow takes an optional thirdreportargument and the one shared notice renders it, so the fork path adds a capability to an existing
surface instead of re-spelling it. Net effect: one error surface where there were two, one
forkErrorstate and one slot-scoped clearing effect removed — the shared notice already clears onslot switch — and the direction-specific copy (the over-capacity refusal naming the direction that
copies fewer rows) is unchanged. The twelve existing
showActionErrorcall sites are untouched,because the new argument is optional.
Provider availability
The row and the path that writes it are provider-agnostic. The signal that ADMITS a
marker is not, and that limit is worth stating plainly rather than implying parity.
General. The event the applier hangs off is
EVENT_TOOL_RESULT, defined in thetransport-neutral ACP layer at
src/kiro_crew/acp/types.py:46and imported by theprovider base contract at
src/kiro_crew/providers/base.py:28. The appliersrc/kiro_crew/dashboard/session_directive_apply.pycarries no provider branch (zerokiro-cli references), and the persisted row is plain
role/content/meta, soany provider that surfaces a tool result reaches the same code.
The limit. The anti-forgery gate honours a directive only when the call was
observed as MCP-served, keyed on
_meta.kiro.toolNamewith_meta.kiro.mcpServerNameset (
src/kiro_crew/session_directive.py:30-40). That identity arrives on kiro-cli'sout-of-band
_metachannel and the gate FAILS CLOSED when it is absent, so on aprovider exposing no equivalent channel a marker is IGNORED rather than mis-applied.
The mechanism is general today; the trust signal is kiro-cli-specific. Generalising it
means giving other providers a provenance channel of their own, which is a larger
change than this PR and is deliberately not attempted here.
What changed
A new transcript row type that renders as a labelled horizontal rule.
A new role,
section_marker, carrying its label inmeta. Not aclson anexisting role: the dashboard serializer persists
clsonly whenrole == "system"whileConversationLog.appendpersists any truthycls, so acls-carried label is silently dropped on one of the two write paths.
metapersists for every role.
contentcarries a human-readable— End of: … —string as the compatibility surface, so an older client degrades legibly rather
than showing a blank row. The role stays out of
RECALL_ROLES(never enters amodel's prompt),
_QUESTION_RETIRING_ROLES(must not retire a pending questioncard) and
_PROMPT_ROLES(must not make a session look freshly asked-of); allthree exclude it by default and tests pin that none gains it.
An MCP tool and session directive
section_marker(label?),wired the way
reset_conversationis: the stateless tool validates and returnsan encoded directive, and the session-aware consumer decodes it at the
tool-result event and applies it against its own slot, so cross-session
misattribution is unrepresentable.
labelis capped at 120 chars — a structuralrow must not become a route for smuggling a paragraph of prose. Dashboard-only,
because the marker is a rendering and a messaging channel has no transcript
surface to draw on.
The append is deferred to a turn boundary, reusing
/note's existing hold.This is the part worth reading the code for. A turn builds its prompt with
exclude_last_n=1so the current turn's user message is not fed back as history,and that exclusion is a raw positional slice applied before role filtering. So
a row appended after the current-turn user row becomes the physical tail, absorbs
the exclusion, and the user's message survives the slice and is replayed —
sending the request twice. Keeping the role out of
RECALL_ROLESdoes notrescue this: membership governs whether the row itself is replayed, never which
row the positional slice removes.
/notedefers its visible line for exactlythis reason, so a marker rides
slot._deferred_notesand the seams that alreadydrain it rather than introducing a third notion of "held". Deferral is arguably
correct anyway: a marker held to the turn's end lands after the closing message
for the item just finished and before anything for the next one. The honest
limit, stated in the tool description: a marker separates turns, so several
emitted in one turn clump at that turn's end.
flush_deferred_notesgained an optional per-entryrole/meta, so a/noteentry keeps its exact prior shape while a marker supplies its own.
noteSessionis stamped only on note rows — it is the surviving half of the
/notewirecontract (
isNoteRowreturns true for any row carrying it, becauseclsdoesnot survive the write path for a non-system role), so stamping it on a marker
would make that predicate call the marker a note.
A renderer registered in both frontend paths, sharing one component. The two
transcript surfaces disagree on the unknown-role fallback:
ChatPage's localrole chain falls through to an assistant markdown bubble, while the SDK path
resolves to
undefinedand draws nothing. Miss one and the row is silently wrongon that surface — a stray bubble, or an invisible row. Both now dispatch to the
same
SectionMarkerRow, so they cannot drift in appearance. EVERY break also draws therow's own time. An unlabelled one needs it because they all render the same fixed caption,
and a labelled one needs it too: the same label is emitted twice whenever one unit of work
is marked more than once (an agent retry), so the caption cannot separate those either.
The time is the only field that differs in both cases.
Also updates the RFC's front matter (
draft→in-progress) and its row in thedirectory index, which the index's own "Keeping this honest" section requires in
the implementing PR.
It also gives
section_markera capability bullet insrc/kiro_crew/config/prompt.md, theagent prompt that ships in the wheel. Nine of the ten
DIRECTIVE_TOOLSmembers already hadone and this did not, so the agent was never told the tool exists — a discoverability gap
rather than a doc nit, since availability is what that file conveys. A new membership-only
assertion in
test_directive_tools_doc_parity.pypins it, so the next directive tool addedcannot miss its bullet silently; the bullet's wording is deliberately not pinned.
Headless producers are refused even when a dashboard tab is open. A cron turn can
run on a user's slot and a sub-agent can share its parent's, so the surface gates are
not sufficient on their own: they ask whether a tab or user surface exists, which a
headless producer riding a user's slot satisfies without a human having asked for
anything.
section_markertherefore joins_USER_ORIGIN_DIRECTIVES, which demandsauthenticated-human provenance for the turn on top of whichever surface gate already
applies — so a marker cannot be written into a human's transcript on the strength of a
tab it merely inherited.
Also tightened:
suggest_followupnow requires human provenance. This is abehaviour change to a pre-existing tool, called out here rather than left to the
diff, because a reviewer or anyone bisecting a broken cron-followup workflow gets
no signal from the
section_markerframing above.What changes.
_USER_ORIGIN_DIRECTIVESis created by this PR and carriessuggest_followupalongsidesection_marker. Before this PR the provenance gateapplied only to
_USER_SURFACE_DIRECTIVES(set_project,reset_conversation),and
suggest_followupwas admitted onhas_dashboard_surfacealone. A headlesscron or sub-agent turn running on a session with an INHERITED dashboard tab could
previously post a follow-up card into that session; after this merge it is refused.
Why it belongs here, and why it is intended. The tool already promised this. On
the base,
mcp_tools/control.pyreads "Restrictions: dashboard sessions only(Slack, cron, and subagent contexts are rejected)", and the surface-only gate did
not enforce that promise for a headless turn with an inherited tab. So the
tightening makes the documented contract true rather than adding a new
restriction. Closing the surface-versus-provenance gap for
section_markerwhileleaving the sibling that makes the same promise unenforced would leave
_DASHBOARD_ONLY_DIRECTIVESinternally inconsistent. Covered bytest_mcp_followup_dispatch.py, and described insession.mdandfollowup-suggestions.md.ask_question, the third member of that set, is deliberately NOT included: itsdescription promises a degradation off the dashboard (an
[OPTIONS:]steer)rather than a refusal, so its gate stays surface-based.
The label is scrubbed by both redactors before the length cap, not after. Ordering
matters and is deliberate: truncating first can cut a credential mid-pattern so the
regex no longer matches, leaving a partial secret in both the
contentandmetasurfaces. Redacting first replaces it whole. Redaction can lengthen a value (a 20-char
key becomes a 22-char tag), so the cap is applied to the scrubbed string and still
binds.
contentandmeta['label']are both derived from that one scrubbed value, sothe drawn field carries the same guarantee as the fallback rather than a weaker one.
Tests
New
test/test_section_marker_directive.py(28 tests) pluswebsite/src/test/SectionMarkerRow.test.tsx, covering the RFC's Phase 1 exitcriteria:
two hairline rules and
role="separator", the SDK registry resolving to itsown entry (not
undrawn, notundefined), andChatPagedispatching on therole and drawing the same component.
meta, asserted against the realflush_deferred_noteson a real slot rather than a stand-in — the appliertests use a fake, which would share a blind spot with the code if the real
flush dropped
roleormeta.only when a seam drains the hold. An idle slot gets it immediately, since there
is no positional tail to steal.
build_session_replayand_recall_rowsreturn no marker content, with thesurrounding user/assistant rows as a positive control so a replay that returned
nothing at all cannot pass.
deferred_notes_fullcondition/note's 429 body does. The directive path hasno HTTP response to carry a status code — the route is Phase 4 — so the
refusal is a string.
/noteentry still writes as aninjectwithreconcile-noteand itsnoteSessionstamp; a 120-char label is accepted sothe 121-char rejection is about the cap and not about long labels; a label is
rendered as text, never as markup.
Reverting
slot_buffers.pyalone makes the two marker-flush assertions fail fortheir intended reasons (
'inject' == 'section_marker', and thenoteSessionstamp) while the
/notecontrol still passes, so the flush test detects thischange specifically rather than any change.
Backend: 341 passed across the touched surface (the new file plus the directive,
note-deferral, flush-safety, replay and driver-directive suites).
isort,flake8andmypyclean; the five touched files are already outsideblack'sbaseline on unmodified
main, and the lines added here areblack-clean.The frontend suite runs in this worktree, and the tests named above were
executed against the change. The parity contract in
chatRolesParity.contract.test.tswas additionally traced against its ownextractor at source — it scrapes
.role [!=]== '<literal>'fromChatPage.tsx,which this change deliberately does NOT add: the row is drawn by the shared
renderer registry, so
ChatPage.tsxcarries nosection_markerdispatch at alland
SectionMarkerRow.test.tsxasserts that absence. The registry claims therole, so both the missing-role and orphaned-role assertions hold.
Screenshots
The fork-failure banner is captured separately by
website/scripts/capture-fork-failure-banner.mjs, whose scene(
website/capture/fork-failure-banner.tsx) mounts the realErrorNoticeand reads itscopy from the live i18n catalog, laid out between a transcript tail and a composer so
the frame shows where the banner lands. Six frames cover both copies: the
over-capacity refusal in each fork direction (the advice differs — an EARLIER message
for a head fork, a LATER one for a tail fork) and the generic failure, in both themes.
The runner asserts one
role="alert"and one agent hand-off per scene and prints thebanner-to-composer gap (16px) before shooting, so a scene that did not render writes no
frame rather than a misleading one.
Captured from the shipped
SectionMarkerRowrendered with the app stylesheet viawebsite/scripts/capture-transcript-section-markers.mjs(Playwright, both themes,deviceScaleFactor 2). The harness at
website/capture/transcript-section-markers.tsxmounts the real component — the same one
ChatPageand the SDK message registrydispatch to — and the capture script asserts four marker rows are present with the
expected label before shooting, so a frame cannot photograph an empty container.
Each frame shows the four states a reviewer needs to judge: a labelled break between
two units of work, an unlabelled break, which has only its time to go on, a label at the 120-char schema cap, and that
same label in a narrow column, where it wraps between the rules instead of
overflowing.
Dark
Light