Skip to content

fix(runtime-host): admit structured-only Messages and keep them model-visible - #4815

Open
ggbdpq wants to merge 14 commits into
apache:mainfrom
ggbdpq:fix/structured-only-message-admission
Open

fix(runtime-host): admit structured-only Messages and keep them model-visible#4815
ggbdpq wants to merge 14 commits into
apache:mainfrom
ggbdpq:fix/structured-only-message-admission

Conversation

@ggbdpq

@ggbdpq ggbdpq commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #4804. Quote-only and attachment-only turns carried real model-facing context but were rejected twice on the way to the model, both times by a text-only admission assumption:

  1. Host admissiondecodeMessageAdmissionContent threw Invalid Message text for empty inline text unless skill ids were present, so a staged quote or a lone attachment could not be submitted.
  2. Replay visibilityruntimeEventHasModelVisibleContent counted only inline text length, so a persisted user event with empty text and a non-empty quotes array was dropped as blocking-unsupported, producing the durable current-run projection is not replayable / empty_text_skipped failure the issue records.

Now decodeMessageAdmissionContent applies the rule itself after structural decode — empty inline text is admissible when the Message carries quotes or attachments, and a Message with none of the three still throws the same invalid-frame error — and the visibility predicate counts structured user context (quotes/attachments) as model-visible. All turn/message admission call sites share the one function, and the model projection already renders quotes (formatQuoteRefs), so structured-only turns now reach the model with their content.

Verification

Claim Command Result
New tests pass with the fix node --test --test-name-pattern="#4804" on core runtime-event + runtime-host protocol dist tests 2 tests, 2 pass
Red-green stash the two production files, rebuild, rerun 2 tests, 2 fail; restored → pass
Quote-only admission at the frame level turn.message.submit with {text:'', quotes:[…]} / {text:'', attachments:[…]} admitted; {text:''} alone still Invalid Message text
No admission regression protocol.test.js (78), message-coordinator.test.js (73) all pass
Format npm run format:check exit 0

Known Windows-environment failures encountered while verifying, both reproduced identically with these production files stashed (control runs) and therefore pre-existing: hosted linked child roots share admission… (EBUSY unlink of a temp runtime.sqlite) and one EPIPE shutdown test in host-kernel. The full Runtime Host suite additionally stalls on this machine on an unrelated file; CI on Linux/macOS is the authoritative full-suite run.

Scope note (updated after the follow-up review): the send path now carries structured content end to end — the Composer send/disable/send-stop gates accept staged quotes and attachments, the Desktop send guard admits them (attachment state, ownership, and size checks unchanged), and the recap renders the actual quoted excerpt via the shared inline-ref formatter instead of a count placeholder. The only remaining cosmetic gap is the transcript display of a quote-only message, which may still render a slim bubble beside the quote chip.

AI use

Implemented with ZCode (GLM-5.3-Flash): traced the issue's four named boundaries to the two blocking ones, applied the smallest contract change at each, and added red-green tests tagged with the issue number. The commit carries the Generated-by trailer.

Checklist

  • Root-cause fix at shared admission/visibility points, not per-surface patches
  • Regression tests fail on the old code (verified via stash/rebuild)
  • Test style follows each file's existing conventions, issue number in test names
  • No new dependencies; renderer display left as a stated follow-up

@github-actions github-actions Bot added the effort/S Under 100 readable lines label Sep 4, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed current head de6183d3172116b220f5fc7b5bbfdf1f3ebc9a59 (OPEN). One P1 below plus a hard CI blocker — this PR is not awaiting human sign-off; its own required check is red.

P1 — admission now accepts empty-text-with-quote, but the two snapshot decoders still reject it

turn.ts:463 (decodeMessageAdmissionContent) parses with allowEmptyText = true and then accepts empty inline text when a quote or attachment is present. But the two places that read these messages back were not changed: message.ts:654 (decodeMessageQueueEntrySnapshot) and session-continuity.ts:778 (steering message events), both importing decodeMessageContent from ./turn.js with the default allowEmptyText = false, which throws Invalid Message text via requireUtf8String. Concrete path: a turn.message.submit with placement: 'next_turn' carrying empty text + a quote is admitted at message.ts:334, enters the pending queue, and then breaks the entire queue snapshot frame when the host serializes it. Same story for session continuity/replay via the steering path. The PR's own new test uses exactly 'next_turn' placement but only asserts the submit frame decodes, stopping one step short of the snapshot frame. Checked and excluded: the storage layer uses the events.ts:260 decoder which only requires typeof text === 'string', so the asymmetry is confined to these two runtime-host protocol call sites — switch them to the same structured-counts-as-valid rule or extract a shared predicate.

CI is red on this head — protocol epoch not bumped

The repo's own protocol-epoch-check.mjs guard fails: protocol files changed but RUNTIME_HOST_COMPATIBILITY_EPOCH is still 112. Note for sequencing: sibling branches bumping the same number merge without git conflict (see #3313), so whoever merges last must re-bump — worth coordinating with the other protocol PRs in flight.

Checked and found sound

The admission refactor itself is behavior-preserving except for the intended widening (allowEmptyText only feeds the empty check at turn.ts:406, UTF-8/length validation untouched); the frame error string is intentionally unchanged; the widened visibility predicate and admission rule agree with each other; re-checked all 5 non-test callers of runtimeEventHasModelVisibleContent (model-history.ts:566,733, runtime-resume.ts:1106,1238, session-recap.ts:120) — all filter semantics, widening only retains more, never drops.

What I could not judge

The P1 path is derived from decoder imports and defaults, not from running the host — a submit-then-snapshot round-trip test would prove or falsify it, and that test does not exist yet. Whether the desktop UI allows empty-body submit was not checked; the protocol is the contract either way.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

本条结论全部来自 @Opus-Qronos-AstroHan 的审查。我自己没有读这份 diff;我核的是当前 head 有没有漂移。当前 head 是 de6183d,未关闭。一条 P1:准入放宽了但读回的两处没跟着放宽,会带倒整个快照帧;另必需检查红了,纪元号没提。修好再合。

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Follow-up to my review above, new head 44d7c17fa46080ab4858b9a581abf65d67381586 (OPEN). The previous P1 is fixed — and fixed exactly where flagged — and the required test check is green on this head. One new P2 below, on the new approach taken to the epoch question.

P2 — a compatibility declaration replaces the epoch bump; the judgment call deserves a deploy-shape answer

This version does not bump the epoch. Instead it adds protocol-compatible-changes/message-admission-quote-or-attachment-text.json, declaring epoch stays 112 on the grounds that no new frame shape is emitted and no previously valid frame is rejected. I read the guard (scripts/protocol-epoch-check.mjs): it is purely mechanical — declaration epoch must equal the head epoch, files must cover all changed protocol files, reason non-empty. It cannot validate the compatibility claim itself, so the claim's correctness is entirely human judgment — which is what needs reviewing now. The script's own comments state the trade-off direction ("a needless epoch bump costs a number, a missed one …"): it leans toward bumping. My recommendation is to bump: the costs are asymmetric — a wasted number versus two incompatible peers both believing they are compatible, an error invisible at handshake and surfacing only when some user pastes a quote into an empty message and the frame fails to decode. Graded P2 rather than P1 because I cannot evidence how often two different-version clients hang off one host in practice; if maintainers can confirm that scenario is excluded by another mechanism (e.g. client version strongly bound to host), the declaration stands and this drops to an observation. That question belongs to someone who knows the deployment shape.

What I could not judge

The host was not run and no cross-version client experiment was constructed — the P2 failure path is derived from decoder behavior differences across versions; the reasoning is held with confidence, the incidence rate without evidence.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

本条是上一条的后续,同样来自 @Opus-Qronos-AstroHan 的审查,针对新 head 44d7c17。上一轮的 P1 已经修掉。新增一条 P2:用兼容声明代替提纪元,这个判断偏乐观,建议提纪元,除非能确认跨版本场景不存在。修好再合。

@github-actions github-actions Bot added effort/M Under 500 readable lines and removed effort/S Under 100 readable lines labels Sep 6, 2026
@ggbdpq

ggbdpq commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Agreed with the bump. The asymmetry decides it: a wrong "compatible" declaration is invisible at handshake and only surfaces when a mixed-version pair actually exchanges a structured-only message, while a spare epoch number costs nothing. I'll bump the epoch 112 → 113 on this PR and replace the compatibility declaration accordingly.


中文:同意 bump。错误的兼容声明在握手时不可见,只会在新旧版本混跑时以用户可见的解码失败暴露;白花一个 epoch 号反而便宜。这个 PR 会把 epoch 提到 113 并相应调整声明文件。

ggbdpq added a commit to ggbdpq/maka that referenced this pull request Sep 7, 2026
…ache#4804)

The structured-only Message admission was declared a compatible
extension, but the declaration is the weaker side of an asymmetric bet:
a wrong "compatible" claim is invisible at handshake and only surfaces
when a mixed-version pair exchanges the new frame, while a spare epoch
number costs nothing. Upstream also moved the epoch 112 -> 123 since the
declaration was written, which invalidates it outright (the guard
requires declaration epoch == head epoch).

Bump RUNTIME_HOST_COMPATIBILITY_EPOCH 123 -> 124, record the change in
the epoch log, and drop the compatibility declaration. Follow-up to the
P2 review on apache#4815.

Generated-by: GLM-5.3-Flash (ZCode)

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed at b4e3f11a1. One [P2] inline, on the epoch bump rather than on the behaviour change — the behaviour review is still in progress and will follow.

Two facts about this head that bound what the review can say. The branch is CONFLICTING with main, and only one check ran on this commit — test. Other pull requests in this repository were running test, audit and label on the same day, so "green" here covers less than it usually does. Neither is a defect in the change; both are context for how much the current signals are worth.

The [P2] is filed now rather than held, because it is about the conflict resolution that is about to happen, and the mistake it warns against is only available while the conflict is still open.

简体中文

b4e3f11a1 上审查。一条 [P2] 发在行内,针对的是 epoch 抬升而不是行为改动 —— 行为面的审查仍在进行中,随后补上。

关于这个 head 的两个事实,它们限定了本次审查能说什么。 分支与 main 处于 CONFLICTING,且该 commit 上只跑了一项检查 —— test。同一天,本仓库其他 PR 跑的是 testauditlabel 三项,所以这里的「绿」覆盖的范围比通常更窄。这两点都不是本次改动的缺陷,而是「当前这些信号值多少」的背景。

这条 [P2] 现在就发、而不是压到最后,因为它针对的正是即将发生的那次冲突解决,而它所警告的那个错误,只在冲突尚未解决时才来得及避免。


Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

Comment thread packages/runtime-host/src/protocol/index.ts Outdated

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A [P1] inline, and it is of an unusual kind: nothing here is broken by the change — the change does not reach the layer that decides.

Together with the epoch note filed earlier, this is a COMMENT and not an approval. The branch is also still CONFLICTING, and only test ran on this commit where other pull requests in this repository were running three checks the same day.

The model-visibility half of the title holds, and that was checked rather than assumed: the shared visibility decision point still governs, a quote-only user event does reach the model on ordinary replay, and none of the previously hidden shapes became visible. The admission half is the one that never reaches production, and the reason the existing suite does not notice is structural — the tests exercise the decoder, and the refusal happens a layer below it.

Worth stating plainly because it changes what to do next: there is nothing to revert here. The decoder change is a correct first step that stops one hop short, and the fix is to move the rule into one place that all three layers share rather than to undo anything.

简体中文

行内一条 [P1],而且是不常见的一种:这里没有任何东西被这次改动弄坏 —— 是这次改动没有触及那个做决定的层。

连同先前发的那条 epoch 备注,本次是 COMMENT 而非批准。 分支同时仍处于 CONFLICTING,且该 commit 上只跑了 test,而同一天本仓库其他 PR 跑的是三项检查。

标题中「保持对模型可见」的那一半成立,而且是核过而非假定的:共享的可见性决定点仍然管辖一切,一条 quote-only 的用户事件在普通 replay 中确实到达模型,先前隐藏的形状没有任何一种变得可见而「接纳」那一半从未到达生产 —— 现有套件之所以没有察觉,原因是结构性的:测试驱动的是解码器,而拒绝发生在它下面一层。

有一点要说明白,因为它决定下一步该做什么:这里没有东西需要回退。 解码器的改动是正确的第一步,只是在关键的一跳前停下了;修法是把规则挪到三层共用的同一个地方,而不是撤销任何东西。


Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

Comment thread packages/runtime-host/src/protocol/turn.ts Outdated
…-visible

A quote-only or attachment-only turn carried real model-facing context but
was rejected at the Host admission boundary ("Invalid Message text") and,
once persisted, dropped by the replay visibility predicate, which counted
only inline text length. The result was exactly apache#4804: structured-only
sends fail before the provider request, and any that persisted render as
an empty user bubble while the model never sees the quoted content.

- decodeMessageAdmissionContent now decodes the frame structurally and
  applies the text rule itself: empty inline text is admissible when the
  Message carries quotes or attachments; a Message with none of the three
  still throws the same invalid-frame error. All turn/message admission
  call sites share this function, so skill-only and structured-only
  admissions now follow one rule.
- runtimeEventHasModelVisibleContent counts a user-authored text event
  with quotes or attachments as model-visible even when the text is
  empty, so the durable event survives replay and the existing quote
  projection (formatQuoteRefs) reaches the model.

Red-green: both new tests (apache#4804-tagged) fail with the production files
stashed and pass with them restored.

Fixes apache#4804

Generated-by: GLM-5.3-Flash (ZCode)
…ire-compatible

The apache#4804 admission change touches packages/runtime-host/src/protocol/turn.ts
without changing the wire: the Host only accepts strictly more frames (an
empty-text Message that carries a quote or an attachment is admitted), emits
nothing new, and rejects nothing that was valid before. Declare it under
protocol-compatible-changes/ at epoch 112 instead of bumping the epoch, per
the apache#3313 guard's compatible-extension path; the guard passes again on the
merge result against current main.

Generated-by: GLM-5.3-Flash (ZCode)
…mission rule

Review P1 (Astro-Han): decodeMessageAdmissionContent now admits an
empty-text Message that carries a quote or an attachment, but the two
places that read those messages back — the message queue entry snapshot
(message.ts) and the durable steering echo (session-continuity.ts) — still
decoded with the default text-length rule, so one admitted next_turn entry
broke the whole queue snapshot frame at serialization. Both call sites use
the same admission decoder now, and a submit-to-snapshot round-trip test
pins the path the review named. The compatible-change declaration grows by
the two read-back files; the protocol epoch guard stays green at 112.

Generated-by: GLM-5.3-Flash (ZCode)
…ache#4804)

The structured-only Message admission was declared a compatible
extension, but the declaration is the weaker side of an asymmetric bet:
a wrong "compatible" claim is invisible at handshake and only surfaces
when a mixed-version pair exchanges the new frame, while a spare epoch
number costs nothing. Upstream also moved the epoch 112 -> 123 since the
declaration was written, which invalidates it outright (the guard
requires declaration epoch == head epoch).

Bump RUNTIME_HOST_COMPATIBILITY_EPOCH 123 -> 124, record the change in
the epoch log, and drop the compatibility declaration. Follow-up to the
P2 review on apache#4815.

Generated-by: GLM-5.3-Flash (ZCode)
@ggbdpq
ggbdpq force-pushed the fix/structured-only-message-admission branch from b4e3f11 to dcf339e Compare September 7, 2026 15:37

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the updates. Re-reviewed dcf339e0f9ba12312790211e614a8cf46c2f3d0b: the queue/continuity decoder changes and the epoch bump to 126 (current main: 125) are correct. I resolved the superseded epoch thread.

The existing P1 at #4815 (comment) still applies: normalizeRootTurnMessageContent in packages/storage/src/agent-run-store.ts rejects every empty-text input. Root-start and queued-successor admission both reach that durable owner, so quote-only/attachment-only input still cannot form a durable Turn. I have left that thread open rather than duplicating the finding.

Please make meaningful-content validity consistent across protocol admission, durable admission and replay, while retaining each boundary's distinct trust/size checks. Verify submit → durable admission → snapshot/replay → model input; decoder-only examples cannot prove this contract. The separate empty-bubble part of #4804 remains explicitly outside this PR, so the issue should not be considered fully addressed by this slice.

AI-assisted independent source review plus primary cross-check; no new Host subprocess test is claimed. Current hosted test is green, which does not discharge the unmet admission contract.

中文 感谢更新。当前 queue/continuity 解码和 epoch 126(main 125)已正确,旧 epoch 串已解决。但已有 P1 仍在:storage 的持久准入仍无条件拒绝空正文,root start 和 queued successor 都会到达该权威。保留原串,不重复发 finding。请统一协议、持久化准入、重放对有效内容的定义,同时保留不同边界各自的信任和大小校验;验收提交→持久准入→快照/重放→模型输入。Issue 的空 bubble 部分仍在范围外,不能因本切片关闭整个问题。本轮为独立 AI 辅助源码审查与主审核对,未重跑 Host 子进程测试;CI 绿不能证明这个缺口已解决。

Completes the storage half of the structured-only message admission:
normalizeRootTurnMessageContent now uses the shared meaningful-content
predicate (text, quote, or attachment) instead of the text-length rule,
so a quote- or attachment-only Message that passes the protocol decoder
also forms a durable Turn. The compaction estimate counts the structured
envelope (a zero estimate dropped model-visible events from the
history-compact gate), and the session recap projects a carrier marker
for structured-only events instead of losing them.

Generated-by: GLM-5.3-Flash (ZCode)
Pins the apache#4804 admission contract at the durable owner:
quote-only and attachment-only root Turn inputs are admitted, and a
truly contentless input still throws the same frame error. On the
pre-fix base the quote-only and attachment-only cases fail (the
text-length rule rejected them), matching jackwener's end-to-end
reproduction on apache#4815.

Generated-by: GLM-5.3-Flash (ZCode)

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing the durable admission boundary. The SQLite write/reopen and ordinary replay checks now preserve structured-only content. One P2 remains in the recap consumer: counting quotes/attachments does not preserve their meaning. I followed up in the existing review thread rather than opening a duplicate finding.

中文

持久化准入已修,写入重开与普通replay能保留内容。recap仍只有数量、没有实际含义,剩余P2已在原串跟进,没有重复开finding。

AI-assisted exact-head rereview by Codex.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing the durable-admission path. An independent pass on 54a454d1 found a remaining P1 original-scope gap before that path: the Desktop scenario in #4804 still cannot send.

With empty text and a ready quote/attachment, packages/ui/src/composer.tsx disables Send (1451–1457), returns early from sendCurrent (1267–1268), and chooses Stop while streaming (1467). Structured content is carried separately and is not converted into editor text. Even if another caller reaches AppShell's onSend, preload routes ordinary sends through session-local:submit; both that service and the direct submit handler call normalizeSessionSendCommand, which still rejects empty text without skills at permission-response-guard.ts:212, before ingestion or Host admission.

Please close the original Composer→Desktop IPC→Host→durable content chain using the existing MessageContent semantics. Do not inject dummy text to pass a text-only guard. Preserve each trust/attachment validation boundary, but let ready structured content count as content for send/stop readiness and admission. The existing recap P2 is another consumer of the same contract: use formatTextWithInlineRefs rather than the new carrier-count placeholder so the recap sees the actual quote/ref. The empty user bubble requested by the issue also remains; if this intentionally delivers only a Host slice, it should not automatically close the Desktop issue.

This is an acceptance gap, not a newly introduced Desktop regression. Production +77/-10 is not excessive; the missing evidence is a real Desktop send operation with downstream model-visible content, rather than more decoder-only cases.

中文

新确认 P1 是原始需求未闭环,不是本 PR 新增 Desktop 回归:Composer 仍禁用空正文发送、sendCurrent 提前返回,流式期间显示 Stop;即使越过 UI,默认 session-local:submit 和直接 submit 都被 Desktop guard 在 ingest/Host 前拒绝。应沿同一 MessageContent 语义修通 Composer→IPC→Host→持久化,不要注入占位正文绕过。recap 复用真实内容 formatter,删除只有数量的占位分支。空 bubble 也未交付,若只做 Host slice 就不能自动关闭 Desktop issue。体积不是主要问题,测试未覆盖原始入口才是。

AI-assisted review using OpenAI Codex/Astra; evidence checked by the coordinating agent. This is not an independent human review.

…-message-admission

# Conflicts:
#	packages/runtime-host/src/protocol/index.ts
…ly message

The recap previously rendered a count placeholder for a structured-only
message; pin the actual excerpt text so the apache#4804 acceptance (quote
content appears in the recap input) is asserted, not assumed.

Generated-by: GLM-5.3-Flash (ZCode)

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for closing the durable-admission and recap gaps. Reviewed b12ce2e6 against #4804 and the downstream consumers. The main Host → SQLite → replay/model/recap path now preserves structured-only content; I did not find a second execution authority.

Two P2 acceptance gaps remain: the shared Composer enables a side-chat operation its caller still rejects (inline), and #4804's quote-only rendering requirement remains incomplete. UserMessageBody in packages/ui/src/chat-turn.tsx:253–264 still creates ChatMessageBubble unconditionally for empty text, including the normal transient/durable quote-only paths. The PR body already acknowledges this; please finish that acceptance item or explicitly narrow the closing claim rather than closing the whole issue with it outstanding.

The branch currently conflicts with main in the compatibility epoch; both branches independently use 131. Please reconcile/rebump against current main. Current-head CI is green, but does not validate that integration. Actual renderer acceptance remains outstanding, so this is not a UI approval.

AI-assisted review with an independent reviewer pass and primary-agent source reconciliation; no new renderer test run.

中文

感谢补齐持久化准入与 recap。主 Host → SQLite → 回放/模型/recap 路径已保留结构化内容,没有发现第二执行权威。

还剩两个 P2 验收缺口:共享 Composer 已启用 side-chat 发送,但调用方仍拒绝(见行内);另外 UserMessageBody 对空文本仍无条件创建气泡,#4804 明列的纯引用渲染要求尚未完成,正文也已承认。请补齐,或明确缩小关闭 issue 的声明。

当前分支与 main 的 compatibility epoch 冲突,两边都独立用了 131,需要收敛并重新验证。当前 CI 绿色不代表冲突解决后的集成结果;尚未完成真实 renderer 验收,因此不作 UI approve。此次由 AI reviewer 独立检查并经主审对照源码复核。

Comment thread packages/ui/src/composer.tsx
…-message-admission

# Conflicts:
#	packages/runtime-host/src/protocol/index.ts
@github-actions github-actions Bot added effort/L Under 1000 readable lines and removed effort/M Under 500 readable lines labels Sep 8, 2026
Review follow-up on apache#4815. The shared Composer now enables Send with an
empty draft once a quote or attachment is staged, but two consumers still
rejected the structured-only frame on the normal user path:

- `useQuoteCompanion.send` gated on `!trimmed` before fork/Host admission,
  so "select transcript text -> Ask about selection -> empty draft -> Send"
  (and attachment-only sends) returned false before reaching the quote and
  attachment payload the send already carries. The same text-only guard in
  `steer` rejected the action while streaming. Both entries now accept an
  empty text when a quote or attachment is staged; steering passes quotes
  and attachment items through the one Message admission channel, and the
  staged quotes stay pending until the Host admits the steering Message.
- `UserMessageBody` in packages/ui created `ChatMessageBubble`
  unconditionally, so a quote-only message rendered an empty bubble on
  both the transient and durable paths. The bubble now renders only for
  non-blank text; quotes and attachments keep their existing surfaces.

The merge with current main also reconciles the compatibility epoch:
both branches had independently claimed 131, so this branch now carries
132 for the structured-only admission widening.

Generated-by: GLM-5.3-Flash (ZCode)
@ggbdpq

ggbdpq commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 2f68f36: the branch now merges current main and carries compatibility epoch 132 — both sides had independently claimed 131, and the merge resolution keeps both epoch-log entries.

The two P2 acceptance gaps are closed on the same push:

  • Side-chat caller: useQuoteCompanion.send and steer no longer reject an empty draft when a quote or attachment is staged — the send reaches the fork/Host admission with its quote snapshot and attachment payload, and steering passes quotes/attachments through the one Message admission channel, with the staged quotes kept pending until the Host admits the steering Message. A regression through the side-chat entry covers both the idle-send and streaming-steer paths.
  • Quote-only rendering: UserMessageBody renders ChatMessageBubble only for non-blank text, so the transient and durable quote-only paths no longer produce an empty bubble; the desktop: quote-only and attachment-only messages require inline text #4804 acceptance item is finished rather than narrowed.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the follow-up. At 2f68f36c8, the earlier structured-only side-chat rejection is fixed, and the quote-only bubble is now omitted. I resolved the superseded thread. One recovery-path P2 remains inline: attachment cleanup must use the same confirmed admission boundary as quote cleanup.

The core fix remains at the existing admission and replay owners; no second durable authority was found. Validation: 58 focused tests passed through a temporary exact-head source loader (storage admission 7, recap 7, quote rendering 2, companion 42). This reused installed dependencies, not a fresh workspace build or Electron visual acceptance. Current-head CI is green, but the PR currently has merge conflicts; refresh the protocol epoch against main when rebasing.

AI-assisted independent reviews and primary source/probe reconciliation.

中文

感谢更新。2f68f36c8 已修复 side-chat 拒绝纯结构化内容的问题,也去掉了 quote-only 空气泡,旧串已解决。还剩一条恢复路径 P2:附件清理应与引用一样以确认准入为边界。

核心修复仍在现有准入与重放权威,没有发现第二个持久权威。临时 exact-head 源码加载验证 58 项通过(storage 7、recap 7、引用渲染 2、companion 42);复用已安装依赖,不是全新构建或 Electron 视觉验收。CI 绿色,但当前分支存在冲突,rebase 时也需对照 main 刷新 epoch。经 AI 独立评审与主审源码/探针核对。

? toComposerIngestItems(pendingAttachments)
: undefined,
);
if (accepted) clearSubmittedAttachments(pendingAttachments);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Keep attachments pending until Host admission is confirmed. On the supported reconnect/failure path, the adapter can return outcome_unknown, but useQuoteCompanion.steer() still returns true when no admission receipt has arrived. This new cleanup then removes the staged attachments although the Message may not have been admitted. Quotes already wait through consumeOnAdmission, so the same structured Message gets two different cleanup boundaries. Please tie attachment consumption to that existing admission/reconciliation path, retaining the pending identity and attachment snapshot while the outcome is unknown; a blind resend with a new identity is not a safe substitute. Cover unknown → late admitted and unknown → retracted, asserting that attachments are consumed only on confirmed admission.

中文

[P2] 请等 Host 确认准入后再清理附件。合理的断线/重连路径会返回 outcome_unknown,但 useQuoteCompanion.steer 在尚无准入回执时仍返回 true,导致这里提前清掉附件,而 Message 可能还没获准入。引用已通过 consumeOnAdmission 等待确认,同一条结构化消息不应使用不同清理边界。请复用现有准入/恢复路径,在未知期间保留 pending identity 和附件快照,不能简单换新 identity 重发。补 unknown→late admitted、unknown→retracted 两种回归,确认只有获准入才消费附件。

# Conflicts:
#	packages/runtime-host/src/protocol/index.ts
#	packages/ui/src/composer.tsx
Review follow-up on apache#4815. `useQuoteCompanion.steer()` returns true on an
`outcome_unknown` result — the supported reconnect/failure path answers
without an admission receipt — and the panel then cleared the submitted
attachments although the Message may never have been admitted. Quotes
already waited through `consumeOnAdmission`, so one structured Message
had two different cleanup boundaries.

Both `send` and `steer` now take an `onAdmitted` callback that fires from
the shared admission boundary: confirmed admission binds the Turn and
consumes the staged quotes and submitted attachments together; an unknown
outcome keeps everything staged until the reconciliation binds the Turn
(a late admission fires the callback then) or a retraction releases the
Message with the attachments still staged for retry. The panel no longer
clears attachments on the optimistic return.

Generated-by: GLM-5.3-Flash (ZCode)
@ggbdpq

ggbdpq commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 11aa81e: the branch merges current main and carries compatibility epoch 134main had claimed 132 (Tool Result ledger refs) and 133 (WorkHub action fields) since the last push, so the merge keeps both upstream entries and re-numbers the structured-only admission widening to 134. protocol-epoch-check --base upstream/main reports 133 -> 134, exit 0.

The composer merge conflict resolves to the union of the two contracts: staged quotes send unconditionally (#4804), while attachment-only sends stay on the host opt-in (allowAttachmentOnlySend, #5003), and the Desktop chat shell now passes that opt-in so the attachment half of #4804 keeps working there.

The new [P2] is also fixed in the same head: send/steer take an onAdmitted callback that fires only from the shared admission boundary, so submitted attachments are consumed together with the staged quotes on confirmed admission. An outcome_unknown steer keeps the attachments staged — a late message_admission fires the callback at the reconciliation, a retraction releases the Message with the attachments still staged for retry — and the panel no longer clears anything on the optimistic return. Regressions cover unknown → late admitted and unknown → retracted; reverting the hook change turns both red.

Verification: epoch guard exit 0; quote-companion suites 56/56; ui composer/chat-turn suites 24/24; format:check clean. CI on this head is re-running.

@ggbdpq

ggbdpq commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 816e6dc (plus merge fc57e13 with current main, #5001 included): fixes the renderer-architecture failure on the previous head. The regression test now takes WorkbarIngestInput through the workbar testing entry instead of a deep ports.js import, and the attachment-only opt-in moved from the frozen app-shell.tsx token budget to the side-chat panel — the surface where the #4804 acceptance scenario actually sends. The main chat shell follows the upstream #5003 default. Architecture check passes against upstream/main; quote-companion 56/56, ui 24/24, format clean.

The CI run on 11aa81e failed the renderer-architecture gate twice:
the regression test imported `WorkbarIngestInput` straight from
`ports.js`, which only `index`/`testing` re-exports may reach from
feature code, and the `allowAttachmentOnlySend` line grew the frozen
`app-shell.tsx` token budget by one.

The type now ships through the workbar `testing.js` entry, and the
side-chat panel — not the frozen shell — opts into attachment-only
sends, which is where the apache#4804 acceptance scenario actually sends
from. The branch also merges the current `main` (apache#5001 included), so
the frozen-file budget is evaluated against the live baseline.

Generated-by: GLM-5.3-Flash (ZCode)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

desktop: quote-only and attachment-only messages require inline text

3 participants