Skip to content

Commit 08afdb1

Browse files
authored
Merge branch 'main' into acp-mode-indicator
2 parents 191f5f3 + a8aae31 commit 08afdb1

12 files changed

Lines changed: 285 additions & 79 deletions

File tree

.github/workflows/docs.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ validate -> test -> build-native (matrix: 4 targets) -> stage-npm -> create-next
5858
- Main branch `@next` publishes reuse the exact same version detection, tag creation, npm publish, and GitHub Release creation code paths as manual `publish_next` dispatches
5959
- The `dry_run` input only applies to `workflow_dispatch` -- tag pushes and main branch pushes always publish for real
6060
- Build runners use Blacksmith (e.g., `blacksmith-4vcpu-ubuntu-2404`) for most jobs, with standard `ubuntu-24.04` for npm publish (which needs the `npm-publish` environment)
61+
- Rust CI caches Cargo registry and git dependency data, but intentionally does not cache `~/.cargo/bin/`; restoring cached binaries after toolchain setup can overwrite rustup-managed `cargo` shims on hosted runners.
6162
- Git tags are the source of truth for all version numbering -- both the `validate` job (via `create_nori_release --get-next-version`) and the `create_nori_release` script's `determine_version()` function use `list_tags()` to enumerate existing versions; GitHub Releases are not consulted for version counting
6263

6364
Created and maintained by Nori.

.github/workflows/rust-ci.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ jobs:
6363
echo "hash=${{ hashFiles('nori-rs/Cargo.lock') }}" >> "$GITHUB_OUTPUT"
6464
echo "toolchain_hash=${{ hashFiles('nori-rs/rust-toolchain.toml') }}" >> "$GITHUB_OUTPUT"
6565
66-
# Cache cargo home directory (registry, git deps, etc.)
66+
# Cache registry and git deps only. Do not cache ~/.cargo/bin because
67+
# restoring tool binaries after rust-toolchain setup can overwrite
68+
# rustup-managed cargo shims on hosted runners.
6769
- name: Restore cargo home cache
6870
id: cache_cargo_home_restore
6971
uses: actions/cache/restore@v4

nori-rs/acp/docs.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ ACP session-domain state now flows through a single serialized reducer. `Session
7979
`SessionRuntime` is the authoritative model for:
8080
- whether the ACP session is idle, loading, or in a prompt turn
8181
- queued user prompts and compact prompts waiting behind an active request
82-
- request-local message assembly for assistant/reasoning streams
82+
- request-local message assembly for user/assistant/reasoning streams, including flushing the prior open text buffer when the ACP session update type changes
8383
- tool snapshot ownership via `owner_request_id`
8484
- pending permission request ownership and cancellation cleanup
8585
- final assistant message extraction used for `PromptCompleted { last_agent_message, .. }`
@@ -89,7 +89,7 @@ The live backend path in `user_input.rs`, `submit_and_ops.rs`, `spawn_and_relay.
8989

9090
Metadata notifications that ACP permits while idle are treated as session-owned rather than request-owned. `AvailableCommandsUpdate`, `CurrentModeUpdate`, `ConfigOptionUpdate`, `SessionInfoUpdate`, and `UsageUpdate` no longer produce "no request is active" warnings; instead the reducer persists the latest values and forwards normalized `ClientEvent`s downstream.
9191

92-
`session/load` replay also preserves more session context than before. User-side `MessageDelta { stream: User, .. }` values are reassembled into `ReplayEntry::UserMessage`, while `SessionUpdateInfo` notes pass through unchanged. For usage updates, that replay path now restores the structured footer context state without needing to re-render the verbose message in history.
92+
`session/load` replay also preserves more session context than before. User-side `MessageDelta { stream: User, .. }` values are reassembled into `ReplayEntry::UserMessage`, while `SessionUpdateInfo` notes pass through unchanged. Message replay preserves chronological stream-kind boundaries: an answer -> reasoning -> answer sequence becomes three replay entries, while adjacent deltas of the same stream are still coalesced. For usage updates, that replay path now restores the structured footer context state without needing to re-render the verbose message in history.
9393

9494
**Custom Agent TOML Schema** (`config/types/mod.rs`):
9595

@@ -740,6 +740,8 @@ Approval request → client_event entry
740740

741741
Older `tool_call`, `tool_result`, and `patch_apply` transcript entry types remain in the schema for legacy read compatibility, but ACP live recording now uses normalized `ClientEvent` entries so transcript persistence matches the live TUI path.
742742

743+
Reducer-owned transcript assembly preserves ACP session update type boundaries. When text changes from assistant to reasoning, reasoning to assistant, or user to either agent stream, the previous open message is flushed before the new stream is accumulated. Consecutive chunks with the same stream are still treated as one message because stable ACP does not provide a durable same-type message boundary.
744+
743745
Tool output for non-patch `tool_result` entries is truncated to 10,000 bytes when recording to transcript. All string truncation helpers in the crate -- `truncate_for_log()` in `tool_display.rs` (tracing previews), `truncate_str()` in `translator.rs` (tool-call display labels like "Execute: ..."), and the transcript byte truncation -- use `codex_utils_string::take_bytes_at_char_boundary()` to avoid slicing inside multi-byte UTF-8 characters.
744746

745747
Configuration:

nori-rs/acp/src/backend/session_reducer.rs

Lines changed: 66 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,7 @@ fn reduce_permission_request(
598598
// Message assembly
599599
// ---------------------------------------------------------------------------
600600

601+
#[derive(Copy, Clone, PartialEq, Eq)]
601602
enum MessageKind {
602603
Agent,
603604
Thought,
@@ -609,15 +610,17 @@ fn append_chunk_to_open_message(
609610
chunk: &acp::ContentChunk,
610611
kind: MessageKind,
611612
) {
612-
let Some(active) = &mut runtime.active else {
613-
return;
614-
};
615-
616613
let text = match &chunk.content {
617614
acp::ContentBlock::Text(t) => &t.text,
618615
_ => return,
619616
};
620617

618+
flush_open_messages_except(runtime, kind);
619+
620+
let Some(active) = &mut runtime.active else {
621+
return;
622+
};
623+
621624
let open = match kind {
622625
MessageKind::Agent => active
623626
.open_agent_message
@@ -633,6 +636,33 @@ fn append_chunk_to_open_message(
633636
open.chunks.push(text.clone());
634637
}
635638

639+
fn flush_open_messages_except(runtime: &mut SessionRuntime, keep: MessageKind) {
640+
let (user, thought, agent) = {
641+
let Some(active) = &mut runtime.active else {
642+
return;
643+
};
644+
(
645+
(keep != MessageKind::User)
646+
.then(|| active.open_user_message.take())
647+
.flatten(),
648+
(keep != MessageKind::Thought)
649+
.then(|| active.open_thought_message.take())
650+
.flatten(),
651+
(keep != MessageKind::Agent)
652+
.then(|| active.open_agent_message.take())
653+
.flatten(),
654+
)
655+
};
656+
657+
push_open_transcript_message(runtime, TranscriptRole::User, user);
658+
push_open_transcript_message(runtime, TranscriptRole::Thought, thought);
659+
if let Some(text) = push_open_transcript_message(runtime, TranscriptRole::Agent, agent)
660+
&& let Some(active) = &mut runtime.active
661+
{
662+
active.last_agent_message = Some(text);
663+
}
664+
}
665+
636666
// ---------------------------------------------------------------------------
637667
// Active request finalization
638668
// ---------------------------------------------------------------------------
@@ -641,41 +671,43 @@ fn append_chunk_to_open_message(
641671
/// transcript, clear active, and return the last agent message text.
642672
fn finalize_active(runtime: &mut SessionRuntime) -> Option<String> {
643673
let active = runtime.active.take()?;
644-
let mut last_agent_message = None;
645-
646-
// Finalize open messages in order: user, thought, agent.
647-
if let Some(open) = active.open_user_message {
648-
let text = open.text();
649-
if !text.is_empty() {
650-
runtime.persisted.transcript.push(TranscriptMessage {
651-
role: TranscriptRole::User,
652-
content: text,
653-
});
654-
}
655-
}
656-
if let Some(open) = active.open_thought_message {
657-
let text = open.text();
658-
if !text.is_empty() {
659-
runtime.persisted.transcript.push(TranscriptMessage {
660-
role: TranscriptRole::Thought,
661-
content: text,
662-
});
663-
}
664-
}
665-
if let Some(open) = active.open_agent_message {
666-
let text = open.text();
667-
if !text.is_empty() {
668-
last_agent_message = Some(text.clone());
669-
runtime.persisted.transcript.push(TranscriptMessage {
670-
role: TranscriptRole::Agent,
671-
content: text,
672-
});
673-
}
674+
let mut last_agent_message = active.last_agent_message;
675+
676+
// At most one text kind should still be open because kind switches flush
677+
// the previous buffer as chunks arrive.
678+
push_open_transcript_message(runtime, TranscriptRole::User, active.open_user_message);
679+
push_open_transcript_message(
680+
runtime,
681+
TranscriptRole::Thought,
682+
active.open_thought_message,
683+
);
684+
if let Some(text) =
685+
push_open_transcript_message(runtime, TranscriptRole::Agent, active.open_agent_message)
686+
{
687+
last_agent_message = Some(text);
674688
}
675689

676690
last_agent_message
677691
}
678692

693+
fn push_open_transcript_message(
694+
runtime: &mut SessionRuntime,
695+
role: TranscriptRole,
696+
open: Option<OpenMessage>,
697+
) -> Option<String> {
698+
let text = open?.text();
699+
if text.is_empty() {
700+
return None;
701+
}
702+
703+
runtime.persisted.transcript.push(TranscriptMessage {
704+
role,
705+
content: text.clone(),
706+
});
707+
708+
(role == TranscriptRole::Agent).then_some(text)
709+
}
710+
679711
// ---------------------------------------------------------------------------
680712
// Helpers
681713
// ---------------------------------------------------------------------------

nori-rs/acp/src/backend/session_reducer/tests.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,71 @@ fn multiple_chunks_assembled_into_one_transcript_entry() {
615615
assert_eq!(agent_messages[0].content, "hello world!");
616616
}
617617

618+
#[test]
619+
fn mixed_agent_and_thought_chunks_preserve_transcript_order() {
620+
let mut rt = new_runtime();
621+
let mut norm = new_normalizer();
622+
623+
reduce(
624+
&mut rt,
625+
InboundEvent::PromptSubmit(simple_prompt()),
626+
&mut norm,
627+
);
628+
629+
let updates = [
630+
acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(acp::ContentBlock::Text(
631+
acp::TextContent::new("CI is green."),
632+
))),
633+
acp::SessionUpdate::AgentThoughtChunk(acp::ContentChunk::new(acp::ContentBlock::Text(
634+
acp::TextContent::new("Preparing PR."),
635+
))),
636+
acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new(acp::ContentBlock::Text(
637+
acp::TextContent::new("The PR is up."),
638+
))),
639+
];
640+
641+
for update in updates {
642+
reduce(&mut rt, notification(update), &mut norm);
643+
}
644+
645+
reduce(
646+
&mut rt,
647+
InboundEvent::PromptResponse {
648+
stop_reason: acp::StopReason::EndTurn,
649+
},
650+
&mut norm,
651+
);
652+
653+
let transcript: Vec<_> = rt
654+
.persisted
655+
.transcript
656+
.iter()
657+
.map(|message| (message.role, message.content.as_str()))
658+
.collect();
659+
660+
assert_eq!(
661+
transcript,
662+
vec![
663+
(
664+
nori_protocol::session_runtime::TranscriptRole::User,
665+
"hello",
666+
),
667+
(
668+
nori_protocol::session_runtime::TranscriptRole::Agent,
669+
"CI is green.",
670+
),
671+
(
672+
nori_protocol::session_runtime::TranscriptRole::Thought,
673+
"Preparing PR.",
674+
),
675+
(
676+
nori_protocol::session_runtime::TranscriptRole::Agent,
677+
"The PR is up.",
678+
),
679+
]
680+
);
681+
}
682+
618683
// =========================================================================
619684
// 9. Load lifecycle
620685
// =========================================================================

nori-rs/acp/src/backend/transcript.rs

Lines changed: 71 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -68,46 +68,43 @@ pub fn client_events_to_replay_client_events(
6868
client_events: Vec<nori_protocol::ClientEvent>,
6969
) -> Vec<nori_protocol::ClientEvent> {
7070
let mut replay = Vec::new();
71-
let mut user = String::new();
72-
let mut assistant = String::new();
73-
let mut reasoning = String::new();
74-
75-
let flush_buffers = |replay: &mut Vec<nori_protocol::ClientEvent>,
76-
user: &mut String,
77-
assistant: &mut String,
78-
reasoning: &mut String| {
79-
if !user.is_empty() {
80-
replay.push(nori_protocol::ClientEvent::ReplayEntry(
81-
nori_protocol::ReplayEntry::UserMessage {
82-
text: std::mem::take(user),
83-
},
84-
));
85-
}
86-
if !reasoning.is_empty() {
87-
replay.push(nori_protocol::ClientEvent::ReplayEntry(
88-
nori_protocol::ReplayEntry::ReasoningMessage {
89-
text: std::mem::take(reasoning),
90-
},
91-
));
92-
}
93-
if !assistant.is_empty() {
94-
replay.push(nori_protocol::ClientEvent::ReplayEntry(
95-
nori_protocol::ReplayEntry::AssistantMessage {
96-
text: std::mem::take(assistant),
97-
},
98-
));
71+
let mut current_stream: Option<nori_protocol::MessageStream> = None;
72+
let mut current_text = String::new();
73+
74+
let flush_message = |replay: &mut Vec<nori_protocol::ClientEvent>,
75+
stream: &mut Option<nori_protocol::MessageStream>,
76+
text: &mut String| {
77+
let Some(stream) = stream.take() else {
78+
return;
79+
};
80+
if text.is_empty() {
81+
return;
9982
}
83+
84+
let text = std::mem::take(text);
85+
let entry = match stream {
86+
nori_protocol::MessageStream::User => nori_protocol::ReplayEntry::UserMessage { text },
87+
nori_protocol::MessageStream::Answer => {
88+
nori_protocol::ReplayEntry::AssistantMessage { text }
89+
}
90+
nori_protocol::MessageStream::Reasoning => {
91+
nori_protocol::ReplayEntry::ReasoningMessage { text }
92+
}
93+
};
94+
replay.push(nori_protocol::ClientEvent::ReplayEntry(entry));
10095
};
10196

10297
for event in client_events {
10398
match event {
104-
nori_protocol::ClientEvent::MessageDelta(message_delta) => match message_delta.stream {
105-
nori_protocol::MessageStream::User => user.push_str(&message_delta.delta),
106-
nori_protocol::MessageStream::Answer => assistant.push_str(&message_delta.delta),
107-
nori_protocol::MessageStream::Reasoning => reasoning.push_str(&message_delta.delta),
108-
},
99+
nori_protocol::ClientEvent::MessageDelta(message_delta) => {
100+
if current_stream.as_ref() != Some(&message_delta.stream) {
101+
flush_message(&mut replay, &mut current_stream, &mut current_text);
102+
current_stream = Some(message_delta.stream);
103+
}
104+
current_text.push_str(&message_delta.delta);
105+
}
109106
other => {
110-
flush_buffers(&mut replay, &mut user, &mut assistant, &mut reasoning);
107+
flush_message(&mut replay, &mut current_stream, &mut current_text);
111108
if let Some(replay_entry) = replay_entry_from_client_event(&other) {
112109
replay.push(nori_protocol::ClientEvent::ReplayEntry(replay_entry));
113110
} else if should_pass_through_replay_client_event(&other) {
@@ -117,7 +114,7 @@ pub fn client_events_to_replay_client_events(
117114
}
118115
}
119116

120-
flush_buffers(&mut replay, &mut user, &mut assistant, &mut reasoning);
117+
flush_message(&mut replay, &mut current_stream, &mut current_text);
121118
replay
122119
}
123120

@@ -386,6 +383,45 @@ mod tests {
386383
);
387384
}
388385

386+
#[test]
387+
fn client_events_to_replay_client_events_preserves_mixed_message_delta_order() {
388+
let replay = client_events_to_replay_client_events(vec![
389+
nori_protocol::ClientEvent::MessageDelta(nori_protocol::MessageDelta {
390+
stream: nori_protocol::MessageStream::Answer,
391+
delta: "CI is green.".into(),
392+
}),
393+
nori_protocol::ClientEvent::MessageDelta(nori_protocol::MessageDelta {
394+
stream: nori_protocol::MessageStream::Reasoning,
395+
delta: "Preparing PR.".into(),
396+
}),
397+
nori_protocol::ClientEvent::MessageDelta(nori_protocol::MessageDelta {
398+
stream: nori_protocol::MessageStream::Answer,
399+
delta: "The PR is up.".into(),
400+
}),
401+
]);
402+
403+
assert_eq!(
404+
replay,
405+
vec![
406+
nori_protocol::ClientEvent::ReplayEntry(
407+
nori_protocol::ReplayEntry::AssistantMessage {
408+
text: "CI is green.".into(),
409+
},
410+
),
411+
nori_protocol::ClientEvent::ReplayEntry(
412+
nori_protocol::ReplayEntry::ReasoningMessage {
413+
text: "Preparing PR.".into(),
414+
},
415+
),
416+
nori_protocol::ClientEvent::ReplayEntry(
417+
nori_protocol::ReplayEntry::AssistantMessage {
418+
text: "The PR is up.".into(),
419+
},
420+
),
421+
]
422+
);
423+
}
424+
389425
#[test]
390426
fn transcript_to_replay_client_events_preserves_session_update_info() {
391427
let transcript = make_transcript(vec![TranscriptEntry::ClientEvent(ClientEventEntry {

0 commit comments

Comments
 (0)