From 6a4902abcfa70b5b7ec24d74886da3d4896f6458 Mon Sep 17 00:00:00 2001 From: codeaf Date: Wed, 30 Sep 2026 21:57:10 -0400 Subject: [PATCH 1/6] session: stop drawing the skills block as part of a message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Skills suited to this message:` is per-turn skill selection for the model. attachTurnSkillsLocked splices it onto the copy the provider reads in a.messages, and that same array is what shapeEntries walks to build every DisplayEntry — so Transcript and AttachReplay drew the block as though the person had typed it, at the top of every reopened conversation and after every message. Strip the block in shapeEntries, the one place a message becomes a displayed row. The remover matches only a whole trailing render the producer itself would have appended: the lead, "- " entry lines, and the closing conflict line, as a suffix. It shares the lead and the conflict line with the producer so the two cannot drift. The model-bound copy is untouched, and a person who pastes the marker or the closing sentence into their own message keeps every word. #1504 was believed to have fixed this but only proved the journal and store thread clean; the display half was never changed. Assisted-by: CodeAF (ember-1) Co-Authored-By: CodeAF <267109073+agentfield-bot@users.noreply.github.com> --- .../1710-strip-skills-block-from-display.md | 18 +++ internal/plan/contract.go | 8 +- internal/session/agent.go | 8 ++ internal/session/skillturn.go | 38 +++++- .../session/skillturn_displayblock_test.go | 108 ++++++++++++++++++ 5 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 docs/changes/unreleased/1710-strip-skills-block-from-display.md create mode 100644 internal/session/skillturn_displayblock_test.go diff --git a/docs/changes/unreleased/1710-strip-skills-block-from-display.md b/docs/changes/unreleased/1710-strip-skills-block-from-display.md new file mode 100644 index 0000000000..c06c6d9079 --- /dev/null +++ b/docs/changes/unreleased/1710-strip-skills-block-from-display.md @@ -0,0 +1,18 @@ +--- +kind: fixed +title: the skills block no longer prints as part of a person's message +pr: 1710 +surface: [chat] +invalidates: + - "#1504 was believed to have fixed `Skills suited to this message:` printing at the top of a conversation and after messages. It proved the journal and store keep only the typed words — which they do — but the engine's in-memory transcript still carried the block, and every display door (Transcript, AttachReplay, and the replay that redraws an opened conversation) drew it straight from there. The display layer now strips a well-formed trailing block, so the record a surface draws is the person's words only." +--- +`Skills suited to this message:` is per-turn skill selection, context for the model. +`attachTurnSkillsLocked` splices it onto the copy the provider reads in +`a.messages`, and that array is also what `shapeEntries` walks to build every +`DisplayEntry`, so replaying or reopening a conversation drew the block as +though the person had typed it. The strip lives in `shapeEntries` — the one place +a message becomes a displayed row — and removes only a whole trailing render the +producer itself would have appended: the lead header, entry lines beginning +`- `, and the closing conflict line, as a suffix of the message. The model-bound +copy is untouched, and a person who pastes the marker or the closing sentence +into their own message keeps every word. diff --git a/internal/plan/contract.go b/internal/plan/contract.go index 03b4140e92..cb62f8f202 100644 --- a/internal/plan/contract.go +++ b/internal/plan/contract.go @@ -446,6 +446,12 @@ func SkillEntryFromFact(fact store.Fact) SkillEntry { return entry } +// SkillsBlockConflictLine is the final line [RenderSkillsBlock] writes, and the +// one the display layer matches to recognise a well-formed trailing block +// (session's stripTurnSkillsBlock). It is named here so the producer and the +// remover read the same bytes and cannot drift. +const SkillsBlockConflictLine = "Earlier-listed skills win when two skills conflict." + // RenderSkillsBlock renders attached skills as doc lines and shelf paths. // Each skill produces one line: "- []" when both exist, or a // shorter form when only one is available; an agentskills folder's line adds @@ -484,7 +490,7 @@ func RenderSkillsBlock(skills []SkillEntry) string { } buf.WriteString("\n") } - buf.WriteString("Earlier-listed skills win when two skills conflict.") + buf.WriteString(SkillsBlockConflictLine) return buf.String() } diff --git a/internal/session/agent.go b/internal/session/agent.go index 6cdc299c1c..e0a46195c3 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -4827,6 +4827,14 @@ func shapeEntries(messages []ai.Message, journal *sessionFile, indexes ...*prese replyTags = append(replyTags, journal.taskReplyTags(msg)...) } displayText := messageContentText(msg) + // THE MODEL'S SKILLS CONTEXT IS NOT CONVERSATION. It rides the copy in + // a.messages the provider reads, and a transcript that drew it would show + // a list nobody asked for at the top of every reopened conversation and + // after every message (#1504's display half; skillturn.go's + // stripTurnSkillsBlock). The journal and store never held it. + if role == "user" { + displayText = stripTurnSkillsBlock(displayText) + } interrupted, explicitlyHuman := false, false if mark := presentation.of(msg); role == "assistant" && mark != nil { interrupted = mark.Interrupted diff --git a/internal/session/skillturn.go b/internal/session/skillturn.go index b04fa31e55..db90d5fcf3 100644 --- a/internal/session/skillturn.go +++ b/internal/session/skillturn.go @@ -43,6 +43,12 @@ const skillTurnMax = 4 // it does in a task's brief). const skillTurnResolveLimit = store.SkillShelfLimit +// turnSkillsLead introduces the block [turnSkills] splices onto the copy the +// model reads. It lives beside the producer AND the remover +// ([stripTurnSkillsBlock]) so the two cannot drift into disagreeing about where +// the block begins. +const turnSkillsLead = "\n\nSkills suited to this message:\n" + // attachTurnSkillsLocked composes the block for one message the person is // sending and splices it onto what the model reads, under a.mu, at the one // door every person-typed message passes through (agent.go submitUser). It is @@ -131,7 +137,37 @@ func (a *Agent) turnSkills(text string) (string, []string) { if block == "" { return "", nil } - return "\n\nSkills suited to this message:\n" + block, carried + return turnSkillsLead + block, carried +} + +// stripTurnSkillsBlock removes the model-only skills context that +// [attachTurnSkillsLocked] spliced onto a person's message, so every display +// door draws the words they typed and not the list chosen for the model +// (agent.go shapeEntries, the one place a message becomes a DisplayEntry). +// +// THE STRIP IS EXACT AND TRAILING, and that is the whole safety of it. It removes +// only a whole block the producer itself would have appended: the lead is +// present, the conflict line closes the message, the entry lines between begin +// with "- " and the lead is not the message. A person who pastes the marker, or +// the conflict sentence, into a message of their own keeps every word, because a +// fragment that is not a whole trailing render is not the block. +func stripTurnSkillsBlock(text string) string { + at := strings.LastIndex(text, turnSkillsLead) + if at <= 0 { + return text + } + rest := text[at+len(turnSkillsLead):] + if !strings.HasSuffix(rest, plan.SkillsBlockConflictLine) { + return text + } + body := strings.TrimSuffix(rest, plan.SkillsBlockConflictLine) + // A RenderSkillsBlock body is one or more "- " lines, the last one closed by + // a newline before the conflict line. A body that does not begin with an + // entry line, or does not close with that newline, was written by a person. + if !strings.HasPrefix(body, "- ") || !strings.HasSuffix(body, "\n") { + return text + } + return text[:at] } // fillSkillRoom takes at most room names off the retrieved half. It is the one diff --git a/internal/session/skillturn_displayblock_test.go b/internal/session/skillturn_displayblock_test.go new file mode 100644 index 0000000000..ae3866ebdb --- /dev/null +++ b/internal/session/skillturn_displayblock_test.go @@ -0,0 +1,108 @@ +package session + +import ( + "context" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/plan" +) + +// THE DISPLAY DOORS SHOW THE WORDS, NOT THE BLOCK. The skills a turn carries +// are model context ([attachTurnSkillsLocked]); the copy in a.messages keeps +// them because it is also the history the provider reads, so every display door +// — Transcript and AttachReplay, both through shapeEntries — must strip the +// trailing block itself. The journal and store already held the person's words +// alone (#1504 pinned that half); this is the display half it never touched. +func TestTheTranscriptShowsTheWordsWhenTheBlockRidesTheModelCopy(t *testing.T) { + brain := openTestBrain(t) + activeSkill(t, brain, "tool:lint", "checks the lint rules for this repo", "/shelf/lint") + + completer := &scriptedCompleter{steps: []step{ + func(_ context.Context, _ []ai.Message) (*ai.Response, error) { + return textResponse("run the lint check"), nil + }, + }} + agent, _ := newTestAgent(t, completer, func(config *Config) { + config.Memory = brain + }) + + words := "how should I lint this repo?" + events, err := agent.Submit(context.Background(), words) + if err != nil { + t.Fatalf("submit: %v", err) + } + drainSkillsNotice(t, events) + + // THE MODEL STILL READS IT: the strip is a display projection, never a cut + // to the provider-bound copy. + sent := userTextIn(completer.request(0)) + if !strings.Contains(sent, "Skills suited to this message:") { + t.Fatalf("the model's copy lost the block:\n%s", sent) + } + + entries := agent.Transcript() + saw := false + for _, entry := range entries { + if entry.Role != "user" { + continue + } + if strings.Contains(entry.Text, "Skills suited to this message:") { + t.Fatalf("a display entry printed the skills block:\n%s", entry.Text) + } + if entry.Text == words { + saw = true + } + } + if !saw { + t.Fatalf("the transcript never showed the person's words whole: %#v", entries) + } +} + +// IT IS EXACTLY THE BLOCK, OR IT IS THE PERSON'S OWN TEXT. A message containing +// the marker, or even the closing sentence, that is not a whole trailing render +// is left untouched — the reporter pasted this text themselves, and their words +// survive. +func TestTheStripLeavesAnyFragmentThatIsNotTheWholeBlock(t *testing.T) { + rendered := turnSkillsLead + plan.RenderSkillsBlock([]plan.SkillEntry{ + {Name: "lint", Doc: "checks the lint rules for this repo", ShelfPath: "/shelf/lint"}, + }) + cases := []struct { + name string + text string + want string + }{ + { + name: "a real rendered block is removed", + text: "how should I lint this repo?" + rendered, + want: "how should I lint this repo?", + }, + { + name: "the marker with no closing line is the person's", + text: "I saw this at the top:\n\nSkills suited to this message:\nand it confused me", + want: "I saw this at the top:\n\nSkills suited to this message:\nand it confused me", + }, + { + name: "the marker and closing line with words after are the person's", + text: "quote:\n\nSkills suited to this message:\n- nothing\nEarlier-listed skills win when two skills conflict.\nnever mind", + want: "quote:\n\nSkills suited to this message:\n- nothing\nEarlier-listed skills win when two skills conflict.\nnever mind", + }, + { + name: "a closing line with no entry body is the person's", + text: "notes:\n\nSkills suited to this message:\nthe end.\nEarlier-listed skills win when two skills conflict.", + want: "notes:\n\nSkills suited to this message:\nthe end.\nEarlier-listed skills win when two skills conflict.", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + entries := shapeEntries([]ai.Message{textMessage("user", tc.text)}, nil) + if len(entries) != 1 || entries[0].Role != "user" { + t.Fatalf("want one user entry, got %#v", entries) + } + if entries[0].Text != tc.want { + t.Fatalf("display text is\n%q\nwant\n%q", entries[0].Text, tc.want) + } + }) + } +} From 6017ce6a2d466abe553be58fe043e206da8e2569 Mon Sep 17 00:00:00 2001 From: ZeroPoint95 <329227198+ZeroPoint95@users.noreply.github.com> Date: Wed, 30 Sep 2026 22:17:07 -0400 Subject: [PATCH 2/6] changelog: the pull request is #1713 Assisted-by: CodeAF (ember-1) Co-Authored-By: CodeAF <267109073+agentfield-bot@users.noreply.github.com> --- ...ck-from-display.md => 1713-strip-skills-block-from-display.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/changes/unreleased/{1710-strip-skills-block-from-display.md => 1713-strip-skills-block-from-display.md} (100%) diff --git a/docs/changes/unreleased/1710-strip-skills-block-from-display.md b/docs/changes/unreleased/1713-strip-skills-block-from-display.md similarity index 100% rename from docs/changes/unreleased/1710-strip-skills-block-from-display.md rename to docs/changes/unreleased/1713-strip-skills-block-from-display.md From ca15623712d79dc1c2197a93974ad6bdcc4fadab Mon Sep 17 00:00:00 2001 From: ZeroPoint95 <329227198+ZeroPoint95@users.noreply.github.com> Date: Wed, 30 Sep 2026 22:17:16 -0400 Subject: [PATCH 3/6] changelog: the entry's pr field is #1713 Assisted-by: CodeAF (ember-1) Co-Authored-By: CodeAF <267109073+agentfield-bot@users.noreply.github.com> --- docs/changes/unreleased/1713-strip-skills-block-from-display.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changes/unreleased/1713-strip-skills-block-from-display.md b/docs/changes/unreleased/1713-strip-skills-block-from-display.md index c06c6d9079..1a8d86255c 100644 --- a/docs/changes/unreleased/1713-strip-skills-block-from-display.md +++ b/docs/changes/unreleased/1713-strip-skills-block-from-display.md @@ -1,7 +1,7 @@ --- kind: fixed title: the skills block no longer prints as part of a person's message -pr: 1710 +pr: 1713 surface: [chat] invalidates: - "#1504 was believed to have fixed `Skills suited to this message:` printing at the top of a conversation and after messages. It proved the journal and store keep only the typed words — which they do — but the engine's in-memory transcript still carried the block, and every display door (Transcript, AttachReplay, and the replay that redraws an opened conversation) drew it straight from there. The display layer now strips a well-formed trailing block, so the record a surface draws is the person's words only." From 87fe990fcdc33c1e16f0cfbff11475cd78ec1441 Mon Sep 17 00:00:00 2001 From: ZeroPoint95 <329227198+ZeroPoint95@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:17:36 -0400 Subject: [PATCH 4/6] session: strip the skills block by injection provenance, not by matching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #1713 found the suffix match could not tell an injected block from one a person pasted whole into a message of their own — the pasted text was silently truncated on display, most surely on a journal-restored message, which never had an injection at all. attachTurnSkillsLocked now marks the one message it splices with the exact bytes it appended (a memory-only messagePresentation field; the journal keeps only the typed words), and shapeEntries removes exactly those bytes from exactly that message. stripTurnSkillsBlock and plan's SkillsBlockConflictLine go away; the pasted-complete-block regression cases are pinned. Assisted-by: CodeAF (ember-1) Co-Authored-By: CodeAF <267109073+agentfield-bot@users.noreply.github.com> --- .../1713-strip-skills-block-from-display.md | 15 ++-- internal/plan/contract.go | 8 +- internal/session/agent.go | 11 ++- internal/session/presentation.go | 8 ++ internal/session/skillturn.go | 42 ++-------- .../session/skillturn_displayblock_test.go | 84 ++++++++++++++----- 6 files changed, 96 insertions(+), 72 deletions(-) diff --git a/docs/changes/unreleased/1713-strip-skills-block-from-display.md b/docs/changes/unreleased/1713-strip-skills-block-from-display.md index 1a8d86255c..04e55cba40 100644 --- a/docs/changes/unreleased/1713-strip-skills-block-from-display.md +++ b/docs/changes/unreleased/1713-strip-skills-block-from-display.md @@ -4,15 +4,18 @@ title: the skills block no longer prints as part of a person's message pr: 1713 surface: [chat] invalidates: - - "#1504 was believed to have fixed `Skills suited to this message:` printing at the top of a conversation and after messages. It proved the journal and store keep only the typed words — which they do — but the engine's in-memory transcript still carried the block, and every display door (Transcript, AttachReplay, and the replay that redraws an opened conversation) drew it straight from there. The display layer now strips a well-formed trailing block, so the record a surface draws is the person's words only." + - "#1504 was believed to have fixed `Skills suited to this message:` printing at the top of a conversation and after messages. It proved the journal and store keep only the typed words — which they do — but the engine's in-memory transcript still carried the block, and every display door (Transcript, AttachReplay, and the replay that redraws an opened conversation) drew it straight from there. The display layer now strips the block by injection provenance, so the record a surface draws is the person's words only." --- `Skills suited to this message:` is per-turn skill selection, context for the model. `attachTurnSkillsLocked` splices it onto the copy the provider reads in `a.messages`, and that array is also what `shapeEntries` walks to build every `DisplayEntry`, so replaying or reopening a conversation drew the block as though the person had typed it. The strip lives in `shapeEntries` — the one place -a message becomes a displayed row — and removes only a whole trailing render the -producer itself would have appended: the lead header, entry lines beginning -`- `, and the closing conflict line, as a suffix of the message. The model-bound -copy is untouched, and a person who pastes the marker or the closing sentence -into their own message keeps every word. +a message becomes a displayed row — and it reads PROVENANCE, never the text: +`attachTurnSkillsLocked` marks the one message it spliced with the exact bytes +it appended (a memory-only `messagePresentation` mark; the journal keeps only +the typed words, so a restored message carries no mark), and the display takes +off exactly those bytes from exactly that message. The model-bound copy is +untouched, and a person who pastes a whole skills block into a message of their +own keeps every word — suffix matching could not tell the two apart, and this +can. diff --git a/internal/plan/contract.go b/internal/plan/contract.go index cb62f8f202..03b4140e92 100644 --- a/internal/plan/contract.go +++ b/internal/plan/contract.go @@ -446,12 +446,6 @@ func SkillEntryFromFact(fact store.Fact) SkillEntry { return entry } -// SkillsBlockConflictLine is the final line [RenderSkillsBlock] writes, and the -// one the display layer matches to recognise a well-formed trailing block -// (session's stripTurnSkillsBlock). It is named here so the producer and the -// remover read the same bytes and cannot drift. -const SkillsBlockConflictLine = "Earlier-listed skills win when two skills conflict." - // RenderSkillsBlock renders attached skills as doc lines and shelf paths. // Each skill produces one line: "- []" when both exist, or a // shorter form when only one is available; an agentskills folder's line adds @@ -490,7 +484,7 @@ func RenderSkillsBlock(skills []SkillEntry) string { } buf.WriteString("\n") } - buf.WriteString(SkillsBlockConflictLine) + buf.WriteString("Earlier-listed skills win when two skills conflict.") return buf.String() } diff --git a/internal/session/agent.go b/internal/session/agent.go index e0a46195c3..a84c18c357 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -4830,10 +4830,15 @@ func shapeEntries(messages []ai.Message, journal *sessionFile, indexes ...*prese // THE MODEL'S SKILLS CONTEXT IS NOT CONVERSATION. It rides the copy in // a.messages the provider reads, and a transcript that drew it would show // a list nobody asked for at the top of every reopened conversation and - // after every message (#1504's display half; skillturn.go's - // stripTurnSkillsBlock). The journal and store never held it. + // after every message (#1504's display half). The strip is by PROVENANCE, + // never by pattern: only a message attachTurnSkillsLocked marked, and only + // the exact bytes it appended, come off — a skills block the person typed + // or pasted themselves has no mark and keeps every word. The journal and + // store never held it, so a restored message carries no mark either. if role == "user" { - displayText = stripTurnSkillsBlock(displayText) + if mark := presentation.of(msg); mark != nil && mark.SkillsBlock != "" { + displayText = strings.TrimSuffix(displayText, mark.SkillsBlock) + } } interrupted, explicitlyHuman := false, false if mark := presentation.of(msg); role == "assistant" && mark != nil { diff --git a/internal/session/presentation.go b/internal/session/presentation.go index f80179c6e4..48ea9c84ee 100644 --- a/internal/session/presentation.go +++ b/internal/session/presentation.go @@ -10,10 +10,18 @@ import ( // messagePresentation records who a producer meant to address. The provider's // content remains untouched; Text, when supplied, is the exact human portion of // a mixed message. Operational records remain available behind disclosure. +// +// SkillsBlock is provenance of another kind: the exact bytes +// [Agent.attachTurnSkillsLocked] spliced onto the copy of THIS message the +// model reads, so shapeEntries can take them off the display by record rather +// than by pattern — a block the person typed themselves has no mark and keeps +// every word. It is memory only: the journal keeps the typed words, so a +// restored message has no block and nothing to strip. type messagePresentation struct { Audience string `json:"audience"` Text *string `json:"text,omitempty"` Interrupted bool `json:"interrupted,omitempty"` + SkillsBlock string `json:"-"` } // Message identity follows its immutable content allocation, as reasoning repair diff --git a/internal/session/skillturn.go b/internal/session/skillturn.go index db90d5fcf3..7aaddfc3ca 100644 --- a/internal/session/skillturn.go +++ b/internal/session/skillturn.go @@ -44,9 +44,7 @@ const skillTurnMax = 4 const skillTurnResolveLimit = store.SkillShelfLimit // turnSkillsLead introduces the block [turnSkills] splices onto the copy the -// model reads. It lives beside the producer AND the remover -// ([stripTurnSkillsBlock]) so the two cannot drift into disagreeing about where -// the block begins. +// model reads. const turnSkillsLead = "\n\nSkills suited to this message:\n" // attachTurnSkillsLocked composes the block for one message the person is @@ -75,6 +73,14 @@ func (a *Agent) attachTurnSkillsLocked(user *userMessage) { } user.message = textMessage("user", messageContentText(user.message)+block) user.skills = carried + // Provenance for the display door: shapeEntries strips the block by THIS + // mark — the exact bytes appended to this one message — and never by + // matching the text, so a block the person pasted into a message of their + // own is kept word for word. + if a.presentation == nil { + a.presentation = &presentationIndex{} + } + a.presentation.remember(user.message, &messagePresentation{SkillsBlock: block}) } // turnSkills composes the skills one message carries and renders them as the @@ -140,36 +146,6 @@ func (a *Agent) turnSkills(text string) (string, []string) { return turnSkillsLead + block, carried } -// stripTurnSkillsBlock removes the model-only skills context that -// [attachTurnSkillsLocked] spliced onto a person's message, so every display -// door draws the words they typed and not the list chosen for the model -// (agent.go shapeEntries, the one place a message becomes a DisplayEntry). -// -// THE STRIP IS EXACT AND TRAILING, and that is the whole safety of it. It removes -// only a whole block the producer itself would have appended: the lead is -// present, the conflict line closes the message, the entry lines between begin -// with "- " and the lead is not the message. A person who pastes the marker, or -// the conflict sentence, into a message of their own keeps every word, because a -// fragment that is not a whole trailing render is not the block. -func stripTurnSkillsBlock(text string) string { - at := strings.LastIndex(text, turnSkillsLead) - if at <= 0 { - return text - } - rest := text[at+len(turnSkillsLead):] - if !strings.HasSuffix(rest, plan.SkillsBlockConflictLine) { - return text - } - body := strings.TrimSuffix(rest, plan.SkillsBlockConflictLine) - // A RenderSkillsBlock body is one or more "- " lines, the last one closed by - // a newline before the conflict line. A body that does not begin with an - // entry line, or does not close with that newline, was written by a person. - if !strings.HasPrefix(body, "- ") || !strings.HasSuffix(body, "\n") { - return text - } - return text[:at] -} - // fillSkillRoom takes at most room names off the retrieved half. It is the one // place [skillTurnMax] bites: attachments and pins have already taken their // seats, and the guesses fill only what is left. diff --git a/internal/session/skillturn_displayblock_test.go b/internal/session/skillturn_displayblock_test.go index ae3866ebdb..a09b62d6dd 100644 --- a/internal/session/skillturn_displayblock_test.go +++ b/internal/session/skillturn_displayblock_test.go @@ -12,8 +12,8 @@ import ( // THE DISPLAY DOORS SHOW THE WORDS, NOT THE BLOCK. The skills a turn carries // are model context ([attachTurnSkillsLocked]); the copy in a.messages keeps // them because it is also the history the provider reads, so every display door -// — Transcript and AttachReplay, both through shapeEntries — must strip the -// trailing block itself. The journal and store already held the person's words +// — Transcript and AttachReplay, both through shapeEntries — takes the block +// off the row it draws. The journal and store already held the person's words // alone (#1504 pinned that half); this is the display half it never touched. func TestTheTranscriptShowsTheWordsWhenTheBlockRidesTheModelCopy(t *testing.T) { brain := openTestBrain(t) @@ -60,43 +60,81 @@ func TestTheTranscriptShowsTheWordsWhenTheBlockRidesTheModelCopy(t *testing.T) { } } -// IT IS EXACTLY THE BLOCK, OR IT IS THE PERSON'S OWN TEXT. A message containing -// the marker, or even the closing sentence, that is not a whole trailing render -// is left untouched — the reporter pasted this text themselves, and their words -// survive. -func TestTheStripLeavesAnyFragmentThatIsNotTheWholeBlock(t *testing.T) { +// THE STRIP READS PROVENANCE, NEVER THE TEXT. Only a message +// [attachTurnSkillsLocked] marked — and only the exact bytes it appended — +// come off a displayed row. A block the person typed or pasted themselves has +// no mark and keeps every word, however well-formed it is: suffix matching +// cannot tell the two apart, and a restored message never had an injection at +// all (the journal keeps the typed words; the mark is memory-only). +func TestTheStripReadsProvenanceNeverTheText(t *testing.T) { rendered := turnSkillsLead + plan.RenderSkillsBlock([]plan.SkillEntry{ {Name: "lint", Doc: "checks the lint rules for this repo", ShelfPath: "/shelf/lint"}, }) + words := "how should I lint this repo?" + pasted := "\n\nSkills suited to this message:\n" + + "- the sheet as I received it [/elsewhere/SKILL.md — body in this file]\n" + + "Earlier-listed skills win when two skills conflict." + + mark := func(text, block string) (ai.Message, *presentationIndex) { + msg := textMessage("user", text) + index := &presentationIndex{} + index.remember(msg, &messagePresentation{SkillsBlock: block}) + return msg, index + } + injected, injectedIndex := mark(words+rendered, rendered) + both, bothIndex := mark(words+pasted+rendered, rendered) + cases := []struct { - name string - text string - want string + name string + msg ai.Message + index *presentationIndex + want string }{ { - name: "a real rendered block is removed", - text: "how should I lint this repo?" + rendered, - want: "how should I lint this repo?", + name: "the block this session injected comes off", + msg: injected, + index: injectedIndex, + want: words, }, { - name: "the marker with no closing line is the person's", - text: "I saw this at the top:\n\nSkills suited to this message:\nand it confused me", - want: "I saw this at the top:\n\nSkills suited to this message:\nand it confused me", + name: "the same bytes without a mark are the person's own", + msg: textMessage("user", words+rendered), + index: nil, + want: words + rendered, }, { - name: "the marker and closing line with words after are the person's", - text: "quote:\n\nSkills suited to this message:\n- nothing\nEarlier-listed skills win when two skills conflict.\nnever mind", - want: "quote:\n\nSkills suited to this message:\n- nothing\nEarlier-listed skills win when two skills conflict.\nnever mind", + name: "a whole well-formed block and nothing else is still the person's", + msg: textMessage("user", rendered), + index: nil, + want: rendered, }, { - name: "a closing line with no entry body is the person's", - text: "notes:\n\nSkills suited to this message:\nthe end.\nEarlier-listed skills win when two skills conflict.", - want: "notes:\n\nSkills suited to this message:\nthe end.\nEarlier-listed skills win when two skills conflict.", + name: "only the injected copy comes off a pasted one", + msg: both, + index: bothIndex, + want: words + pasted, + }, + { + name: "the marker with no closing line is the person's", + msg: textMessage("user", "I saw this at the top:\n\nSkills suited to this message:\nand it confused me"), + index: nil, + want: "I saw this at the top:\n\nSkills suited to this message:\nand it confused me", + }, + { + name: "the marker and closing line with words after are the person's", + msg: textMessage("user", "quote:\n\nSkills suited to this message:\n- nothing\nEarlier-listed skills win when two skills conflict.\nnever mind"), + index: nil, + want: "quote:\n\nSkills suited to this message:\n- nothing\nEarlier-listed skills win when two skills conflict.\nnever mind", }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - entries := shapeEntries([]ai.Message{textMessage("user", tc.text)}, nil) + var entries []DisplayEntry + if tc.index != nil { + entries = shapeEntries([]ai.Message{tc.msg}, nil, tc.index) + } else { + entries = shapeEntries([]ai.Message{tc.msg}, nil) + } if len(entries) != 1 || entries[0].Role != "user" { t.Fatalf("want one user entry, got %#v", entries) } From 8501fad6e9663cdfefbf73e1a60bc24104d20ead Mon Sep 17 00:00:00 2001 From: ZeroPoint95 <329227198+ZeroPoint95@users.noreply.github.com> Date: Thu, 1 Oct 2026 09:52:24 -0400 Subject: [PATCH 5/6] session: pin the strip at the in-flight replay door and the mark's identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #1713 asked for the two regressions it could not see from the settled record: AttachReplay's own mid-turn slice must keep the strip (nothing exercised that door), and the provenance mark's pointer-identity assumption — recordUserLocked appends user.message without re-allocating Content, unlike recordPresentedAssistant — must be a tripwire, since a deep copy on the user path would drop the mark silently and draw the model's skills block back as the person's own words. Assisted-by: CodeAF (ember-1) Co-Authored-By: CodeAF <267109073+agentfield-bot@users.noreply.github.com> --- .../session/skillturn_displayblock_test.go | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/internal/session/skillturn_displayblock_test.go b/internal/session/skillturn_displayblock_test.go index a09b62d6dd..41e4a53a3b 100644 --- a/internal/session/skillturn_displayblock_test.go +++ b/internal/session/skillturn_displayblock_test.go @@ -144,3 +144,115 @@ func TestTheStripReadsProvenanceNeverTheText(t *testing.T) { }) } } + +// THE IN-FLIGHT DOOR KEEPS THE STRIP TOO. AttachReplay reads the same +// presentation index Transcript does, but it draws the running turn's opening +// message off its own slice ([attachReplayLocked]'s `kept`, copied out of +// a.messages[:floor]), so the provenance mark has to survive THAT copy as well. +// This is the door a surface arrives through mid-turn — the second half of the +// report's "very often when chatting" — and nothing else exercises it: both +// tests above read the settled record through Transcript alone. +func TestAttachReplayMidTurnKeepsTheStrip(t *testing.T) { + brain := openTestBrain(t) + activeSkill(t, brain, "tool:lint", "checks the lint rules for this repo", "/shelf/lint") + + // midTurn is closed inside the turn's one step, so the assertions run while + // a.running and a.hub are both live — the state AttachReplay splits on. + midTurn := make(chan struct{}) + carryOn := make(chan struct{}) + completer := &scriptedCompleter{steps: []step{ + func(_ context.Context, _ []ai.Message) (*ai.Response, error) { + close(midTurn) + <-carryOn + return textResponse("run the lint check"), nil + }, + }} + agent, _ := newTestAgent(t, completer, func(config *Config) { + config.Memory = brain + }) + + words := "how should I lint this repo?" + events, err := agent.Submit(context.Background(), words) + if err != nil { + t.Fatalf("submit: %v", err) + } + <-midTurn + + entries, stream, stop := agent.AttachReplay() + if stream == nil { + t.Fatal("AttachReplay mid-turn handed back no stream") + } + defer stop() + saw := false + for _, entry := range entries { + if entry.Role != "user" { + continue + } + if strings.Contains(entry.Text, "Skills suited to this message:") { + t.Fatalf("the in-flight replay drew the skills block:\n%s", entry.Text) + } + if entry.Text == words { + saw = true + } + } + if !saw { + t.Fatalf("the in-flight replay never showed the person's words whole: %#v", entries) + } + + close(carryOn) + collect(t, events) + collect(t, stream) +} + +// THE MARK IS KEYED TO THE MESSAGE'S CONTENT ALLOCATION, AND THE USER PATH MUST +// NOT MOVE IT. [presentationIndex.remember] keys on &message.Content[0], and +// [Agent.recordUserLocked] appends `user.message` by value WITHOUT +// re-allocating Content — so the address the mark was recorded under is the +// address [shapeEntries] later looks up. The assistant path is the opposite: +// [recordPresentedAssistant] deep-copies Content first, because its callers may +// reuse the value. If a deep copy were ever introduced between +// [attachTurnSkillsLocked] and recordUserLocked's append, the mark would be +// dropped SILENTLY — `of` returns nil, no error is raised, and the model's +// skills block is drawn back as the person's own words. This is the tripwire. +func TestTheUserMessagesProvenanceMarkSurvivesItsAppendToTheTranscript(t *testing.T) { + brain := openTestBrain(t) + activeSkill(t, brain, "tool:lint", "checks the lint rules for this repo", "/shelf/lint") + + completer := &scriptedCompleter{steps: []step{ + func(_ context.Context, _ []ai.Message) (*ai.Response, error) { + return textResponse("run the lint check"), nil + }, + }} + agent, _ := newTestAgent(t, completer, func(config *Config) { + config.Memory = brain + }) + + words := "how should I lint this repo?" + events, err := agent.Submit(context.Background(), words) + if err != nil { + t.Fatalf("submit: %v", err) + } + drainSkillsNotice(t, events) + + // THE RECORDED MESSAGE STILL CARRIES ITS MARK, found by the address the + // append published. A deep copy on the way in would have made this nil. + agent.mu.Lock() + index := agent.presentation + recorded := ai.Message{} + found := false + for _, msg := range agent.messages { + if msg.Role == "user" && strings.Contains(messageContentText(msg), words) { + recorded, found = msg, true + } + } + agent.mu.Unlock() + if !found { + t.Fatal("the person's message is not in the transcript") + } + if mark := index.of(recorded); mark == nil || mark.SkillsBlock == "" { + t.Fatal("the recorded user message lost its provenance mark — the user path must not deep-copy Content") + } + if !strings.Contains(messageContentText(recorded), "Skills suited to this message:") { + t.Fatal("the model's copy in the transcript lost the block") + } +} From e488d226b363517852a1e92f07a57ddcd0e611ae Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Thu, 1 Oct 2026 12:41:03 -0400 Subject: [PATCH 6/6] session: keep skill context out of the person's words Use one provenance helper for transcript display, rewind drafts, Why and conversation title input. Apply it to ordinary and steering messages when compaction re-journals its kept window. Keep the provider copy and pasted blocks intact. Document why older compacted journals still show blocks saved without injection marks. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../1713-strip-skills-block-from-display.md | 26 +- internal/manual/chat/skills-a-turn-used.md | 14 +- internal/session/agent.go | 13 +- internal/session/presentation.go | 22 +- internal/session/rewind.go | 2 +- internal/session/sessionfile.go | 14 +- .../session/skillturn_displayblock_test.go | 23 +- internal/session/skillturn_doors_test.go | 303 ++++++++++++++++++ internal/session/title.go | 2 +- internal/session/why.go | 2 +- 10 files changed, 377 insertions(+), 44 deletions(-) create mode 100644 internal/session/skillturn_doors_test.go diff --git a/docs/changes/unreleased/1713-strip-skills-block-from-display.md b/docs/changes/unreleased/1713-strip-skills-block-from-display.md index 04e55cba40..0f2b6b7864 100644 --- a/docs/changes/unreleased/1713-strip-skills-block-from-display.md +++ b/docs/changes/unreleased/1713-strip-skills-block-from-display.md @@ -1,21 +1,17 @@ --- kind: fixed -title: the skills block no longer prints as part of a person's message +title: skills context stays out of messages, rewind, reopened chats and titles pr: 1713 surface: [chat] invalidates: - - "#1504 was believed to have fixed `Skills suited to this message:` printing at the top of a conversation and after messages. It proved the journal and store keep only the typed words — which they do — but the engine's in-memory transcript still carried the block, and every display door (Transcript, AttachReplay, and the replay that redraws an opened conversation) drew it straight from there. The display layer now strips the block by injection provenance, so the record a surface draws is the person's words only." + - "#1627 was believed to have fixed #1504 (the `Skills suited to this message:` list inside your message). It kept the journal and store to your words, but the live copy still carried the block into reopened conversations, `/export`, rewind drafts, compaction and title input. Now those keep only your words for new messages; the model's copy still carries the block." --- -`Skills suited to this message:` is per-turn skill selection, context for the model. -`attachTurnSkillsLocked` splices it onto the copy the provider reads in -`a.messages`, and that array is also what `shapeEntries` walks to build every -`DisplayEntry`, so replaying or reopening a conversation drew the block as -though the person had typed it. The strip lives in `shapeEntries` — the one place -a message becomes a displayed row — and it reads PROVENANCE, never the text: -`attachTurnSkillsLocked` marks the one message it spliced with the exact bytes -it appended (a memory-only `messagePresentation` mark; the journal keeps only -the typed words, so a restored message carries no mark), and the display takes -off exactly those bytes from exactly that message. The model-bound copy is -untouched, and a person who pastes a whole skills block into a message of their -own keeps every word — suffix matching could not tell the two apart, and this -can. +`Skills suited to this message:` is context for the model. One shared helper +reads the message's injection mark so display, rewind, compaction, the turn's +explanation and title input keep only the person's words. The model's copy keeps +the block, and a block the person pasted keeps every word. + +A conversation compacted by an earlier build may already have the block saved in +its journal. `/export` and rewinding to a message from before the update can still +carry that saved block; the conversation on screen does not show it. Nothing +removes it by matching its wording: a pasted block must keep every word. diff --git a/internal/manual/chat/skills-a-turn-used.md b/internal/manual/chat/skills-a-turn-used.md index 3da946abe5..5d8faa51a8 100644 --- a/internal/manual/chat/skills-a-turn-used.md +++ b/internal/manual/chat/skills-a-turn-used.md @@ -18,10 +18,16 @@ Those names come from the turn's skill list, not by taking apart the words in th line. The row is a record of what that turn carried with it. It is not a warning, a question or work waiting for you, so it has no attention mark, count or action. -The message in the transcript is your words and nothing else. What a turn -carries for the model — the skill bodies it reads with your sentence — rides the -copy the model reads, and no surface prints it: the row above is the one thing -you are shown. +The message in the transcript is your words and nothing else. Rewinding to that +message hands back only your words, too. What a turn carries for the model — the +skill bodies it reads with your sentence — rides the copy the model reads; the +row above tells you which skills it carried. + +A conversation compacted by an older build may already have the skills block +saved as part of its message. `/export` and rewinding to a message from before the +update may still carry that saved block; the conversation on screen does not show +it. codeaf does not remove blocks by their wording, because a block you pasted +yourself must keep every word. ## Did it use my skill? diff --git a/internal/session/agent.go b/internal/session/agent.go index a84c18c357..b1c37e94fb 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -4827,18 +4827,9 @@ func shapeEntries(messages []ai.Message, journal *sessionFile, indexes ...*prese replyTags = append(replyTags, journal.taskReplyTags(msg)...) } displayText := messageContentText(msg) - // THE MODEL'S SKILLS CONTEXT IS NOT CONVERSATION. It rides the copy in - // a.messages the provider reads, and a transcript that drew it would show - // a list nobody asked for at the top of every reopened conversation and - // after every message (#1504's display half). The strip is by PROVENANCE, - // never by pattern: only a message attachTurnSkillsLocked marked, and only - // the exact bytes it appended, come off — a skills block the person typed - // or pasted themselves has no mark and keeps every word. The journal and - // store never held it, so a restored message carries no mark either. + // personWords removes only this message's recorded skills injection. if role == "user" { - if mark := presentation.of(msg); mark != nil && mark.SkillsBlock != "" { - displayText = strings.TrimSuffix(displayText, mark.SkillsBlock) - } + displayText = presentation.personWords(msg) } interrupted, explicitlyHuman := false, false if mark := presentation.of(msg); role == "assistant" && mark != nil { diff --git a/internal/session/presentation.go b/internal/session/presentation.go index 48ea9c84ee..4bb1b59e00 100644 --- a/internal/session/presentation.go +++ b/internal/session/presentation.go @@ -1,6 +1,7 @@ package session import ( + "strings" "sync" "github.com/Agent-Field/agentfield/sdk/go/ai" @@ -15,8 +16,9 @@ import ( // [Agent.attachTurnSkillsLocked] spliced onto the copy of THIS message the // model reads, so shapeEntries can take them off the display by record rather // than by pattern — a block the person typed themselves has no mark and keeps -// every word. It is memory only: the journal keeps the typed words, so a -// restored message has no block and nothing to strip. +// every word. It is memory only: new journal writes keep the typed words, so +// those restored messages need no mark. An older compacted journal can still +// carry the saved block without a mark. type messagePresentation struct { Audience string `json:"audience"` Text *string `json:"text,omitempty"` @@ -58,6 +60,22 @@ func (p *presentationIndex) of(message ai.Message) *messagePresentation { return &mark } +// personWords keeps the person's words separate from the model's skills context. +// THE MODEL'S SKILLS CONTEXT IS NOT CONVERSATION. It rides the copy in a.messages +// the provider reads, and quoting it as the person's message would put words in +// their mouth. The strip is by PROVENANCE, never by pattern: only a message +// attachTurnSkillsLocked marked, and only the exact suffix it appended, comes +// off. A skills block the person typed or pasted themselves keeps every word. +// New journal writes keep these words too; an older compacted journal has no +// injection mark, so its saved bytes remain untouched. A nil index has no marks. +func (p *presentationIndex) personWords(message ai.Message) string { + text := messageContentText(message) + if mark := p.of(message); mark != nil && mark.SkillsBlock != "" { + return strings.TrimSuffix(text, mark.SkillsBlock) + } + return text +} + func humanPresentation(text string) *messagePresentation { return &messagePresentation{Audience: "human", Text: &text} } diff --git a/internal/session/rewind.go b/internal/session/rewind.go index aa8c18d13b..3d8778c1d4 100644 --- a/internal/session/rewind.go +++ b/internal/session/rewind.go @@ -231,7 +231,7 @@ func (a *Agent) rewindPointsLocked() []RewindPoint { if !skipped { point := RewindPoint{Index: index, Turn: turn, Entry: entry} if turn { - point.Said = said + point.Said = a.presentation.personWords(message) } points = append(points, point) } diff --git a/internal/session/sessionfile.go b/internal/session/sessionfile.go index 84cdb84f31..99f3bb0326 100644 --- a/internal/session/sessionfile.go +++ b/internal/session/sessionfile.go @@ -2845,22 +2845,30 @@ func (s *sessionFile) appendCompaction(pass compactionPass, tokensBefore int, wi Timestamp: stamp(), }) for index, message := range window { + // Marks belong to the original message, before projecting a fresh + // journal copy. The live window still carries the model's context. + note := s.isNote(message) + steer := s.steerMark(message) + presentation := s.presentation.of(message) // A KEPT LINE IS RE-JOURNALED AS WHAT IT WAS. The window is written again // on the far side of the marker (above), and a note re-written without its // mark would come back from the next resume as the person's words — this // pass is the one place a message is journaled twice. - if s.isNote(message) { + if note { s.appendNote(message, noteMarks{ tags: s.taskReplyTags(message), deliveries: s.noteDeliveriesOf(message), }) continue } + if message.Role == "user" && presentation != nil && presentation.SkillsBlock != "" { + message = textMessage("user", s.presentation.personWords(message)) + } // AND SO IS A SPLICED ONE, for the same reason: a steer re-written without // its mark would come back from the next resume as a question of its own, // and the turn it was typed into would lose the correction that shaped it. - if mark := s.steerMark(message); mark != nil { - s.appendSteer(message, *mark) + if steer != nil { + s.appendSteer(message, *steer) continue } var reasoning provider.MessageReasoning diff --git a/internal/session/skillturn_displayblock_test.go b/internal/session/skillturn_displayblock_test.go index 41e4a53a3b..6c06a48dd8 100644 --- a/internal/session/skillturn_displayblock_test.go +++ b/internal/session/skillturn_displayblock_test.go @@ -3,7 +3,9 @@ package session import ( "context" "strings" + "sync" "testing" + "time" "github.com/Agent-Field/agentfield/sdk/go/ai" "github.com/Agent-Field/codeaf/internal/plan" @@ -13,8 +15,8 @@ import ( // are model context ([attachTurnSkillsLocked]); the copy in a.messages keeps // them because it is also the history the provider reads, so every display door // — Transcript and AttachReplay, both through shapeEntries — takes the block -// off the row it draws. The journal and store already held the person's words -// alone (#1504 pinned that half); this is the display half it never touched. +// off the row it draws. Ordinary journal lines already held the person's words +// alone (#1410); #1627 pinned the store's copy too. func TestTheTranscriptShowsTheWordsWhenTheBlockRidesTheModelCopy(t *testing.T) { brain := openTestBrain(t) activeSkill(t, brain, "tool:lint", "checks the lint rules for this repo", "/shelf/lint") @@ -64,8 +66,8 @@ func TestTheTranscriptShowsTheWordsWhenTheBlockRidesTheModelCopy(t *testing.T) { // [attachTurnSkillsLocked] marked — and only the exact bytes it appended — // come off a displayed row. A block the person typed or pasted themselves has // no mark and keeps every word, however well-formed it is: suffix matching -// cannot tell the two apart, and a restored message never had an injection at -// all (the journal keeps the typed words; the mark is memory-only). +// cannot tell the two apart. New journal writes keep the typed words, and +// restoring a saved message does not manufacture a memory-only injection mark. func TestTheStripReadsProvenanceNeverTheText(t *testing.T) { rendered := turnSkillsLead + plan.RenderSkillsBlock([]plan.SkillEntry{ {Name: "lint", Doc: "checks the lint rules for this repo", ShelfPath: "/shelf/lint"}, @@ -160,6 +162,8 @@ func TestAttachReplayMidTurnKeepsTheStrip(t *testing.T) { // a.running and a.hub are both live — the state AttachReplay splits on. midTurn := make(chan struct{}) carryOn := make(chan struct{}) + var release sync.Once + resume := func() { release.Do(func() { close(carryOn) }) } completer := &scriptedCompleter{steps: []step{ func(_ context.Context, _ []ai.Message) (*ai.Response, error) { close(midTurn) @@ -170,13 +174,20 @@ func TestAttachReplayMidTurnKeepsTheStrip(t *testing.T) { agent, _ := newTestAgent(t, completer, func(config *Config) { config.Memory = brain }) + // Release before the agent's cleanup so a failed assertion cannot leave + // its provider waiting while Close tries to settle the turn. + t.Cleanup(resume) words := "how should I lint this repo?" events, err := agent.Submit(context.Background(), words) if err != nil { t.Fatalf("submit: %v", err) } - <-midTurn + select { + case <-midTurn: + case <-time.After(10 * time.Second): + t.Fatal("the turn never reached the provider") + } entries, stream, stop := agent.AttachReplay() if stream == nil { @@ -199,7 +210,7 @@ func TestAttachReplayMidTurnKeepsTheStrip(t *testing.T) { t.Fatalf("the in-flight replay never showed the person's words whole: %#v", entries) } - close(carryOn) + resume() collect(t, events) collect(t, stream) } diff --git a/internal/session/skillturn_doors_test.go b/internal/session/skillturn_doors_test.go new file mode 100644 index 0000000000..5fb91f4ea6 --- /dev/null +++ b/internal/session/skillturn_doors_test.go @@ -0,0 +1,303 @@ +package session + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/agentfield/sdk/go/ai" +) + +const skillDoorWords = "how should I lint this repo?" +const skillDoorLead = "Skills suited to this message:" + +// Person-facing words omit only this message's recorded injection. Nil indexes, +// pasted blocks and a changed suffix preserve every word of the model's copy. +func TestPersonWordsRequiresTheMessagesOwnSkillsMark(t *testing.T) { + type personWordReader interface { + personWords(ai.Message) string + } + var absent *presentationIndex + if _, ok := any(absent).(personWordReader); !ok { + t.Fatal("presentationIndex has no shared personWords projection") + } + block := "\n\nSkills suited to this message:\n- lint\nEarlier-listed skills win when two skills conflict." + marked := func(text, injection string) (ai.Message, *presentationIndex) { + message := textMessage("user", text) + index := &presentationIndex{} + index.remember(message, &messagePresentation{SkillsBlock: injection}) + return message, index + } + injected, injectionIndex := marked(skillDoorWords+block, block) + pasted, pastedIndex := marked(skillDoorWords+block+block, block) + changed, changedIndex := marked(skillDoorWords+block+"\nmore words", block) + unmarked, unmarkedIndex := marked(skillDoorWords+block, "") + for _, tc := range []struct { + name string + message ai.Message + index *presentationIndex + want string + }{ + {"nil index keeps pasted bytes", textMessage("user", skillDoorWords+block), nil, skillDoorWords + block}, + {"empty message is safe", ai.Message{Role: "user"}, &presentationIndex{}, ""}, + {"unknown occurrence keeps pasted bytes", textMessage("user", skillDoorWords+block), injectionIndex, skillDoorWords + block}, + {"empty mark keeps pasted bytes", unmarked, unmarkedIndex, skillDoorWords + block}, + {"recorded injection comes off", injected, injectionIndex, skillDoorWords}, + {"pasted copy stays", pasted, pastedIndex, skillDoorWords + block}, + {"recorded bytes must still be the suffix", changed, changedIndex, skillDoorWords + block + "\nmore words"}, + } { + t.Run(tc.name, func(t *testing.T) { + before := messageContentText(tc.message) + if got := any(tc.index).(personWordReader).personWords(tc.message); got != tc.want { + t.Fatalf("person-facing words = %q, want %q", got, tc.want) + } + if messageContentText(tc.message) != before { + t.Fatal("projecting the person's words changed the model's copy") + } + }) + } +} + +func skillDoorAgent(t *testing.T, journal string, replies ...step) (*Agent, *scriptedCompleter) { + t.Helper() + brain := openTestBrain(t) + activeSkill(t, brain, "tool:lint", "checks the lint rules for this repo", "/shelf/lint") + completer := &scriptedCompleter{steps: replies} + agent, _ := newTestAgent(t, completer, func(config *Config) { + config.Memory = brain + config.SessionFile = journal + }) + return agent, completer +} + +func skillDoorReply(context.Context, []ai.Message) (*ai.Response, error) { + return textResponse("run the lint check"), nil +} + +func skillDoorSubmit(t *testing.T, agent *Agent, words string) { + t.Helper() + events, err := agent.Submit(context.Background(), words) + if err != nil { + t.Fatalf("submit: %v", err) + } + for _, event := range collect(t, events) { + if event.Kind == EventError { + t.Fatalf("turn: %v", event.Err) + } + } +} + +func skillDoorRequest(t *testing.T, completer *scriptedCompleter, words string) string { + t.Helper() + // Memory errands share the completer, so the person's message identifies + // the main request without assuming an auxiliary call's position. + for index := completer.requests() - 1; index >= 0; index-- { + for _, message := range completer.request(index) { + text := messageContentText(message) + if message.Role == "user" && strings.HasPrefix(text, words) { + return text + } + } + } + t.Fatalf("no provider request with the person's words %q", words) + return "" +} + +func skillDoorDisplayed(t *testing.T, entries []DisplayEntry) { + t.Helper() + found := false + for _, entry := range entries { + if entry.Role != "user" { + continue + } + if strings.Contains(entry.Text, skillDoorLead) { + t.Errorf("the person's message contains the injected block: %q", entry.Text) + } + found = found || entry.Text == skillDoorWords + } + if !found { + t.Errorf("the transcript does not show exactly the person's words %q", skillDoorWords) + } +} + +// Rewinding hands back only the person's words. Resubmitting that draft keeps +// one block on the model's copy and only the typed words in the journal and store. +func TestRewindDraftKeepsThePersonsWordsWhenSkillsRide(t *testing.T) { + journal := filepath.Join(t.TempDir(), "session.jsonl") + agent, completer := skillDoorAgent(t, journal, skillDoorReply, skillDoorReply) + skillDoorSubmit(t, agent, skillDoorWords) + first := skillDoorRequest(t, completer, skillDoorWords) + if strings.Count(first, skillDoorLead) != 1 { + t.Fatalf("the model's first copy must carry one block: %q", first) + } + var turn *RewindPoint + for _, point := range agent.RewindPoints() { + if point.Turn { + point := point + turn = &point + break + } + } + if turn == nil { + t.Fatal("the submitted message has no rewind point") + } + if turn.Said != skillDoorWords { + t.Errorf("rewind draft contains model context: %q", turn.Said) + } + removed, err := agent.RewindAt(turn.Index) + if err != nil { + t.Fatal(err) + } + skillDoorDisplayed(t, removed) + skillDoorSubmit(t, agent, turn.Said) + sent := skillDoorRequest(t, completer, skillDoorWords) + if sent != first || strings.Count(sent, skillDoorLead) != 1 { + t.Errorf("resubmitting the rewind draft changed the model's copy; blocks=%d: %q", strings.Count(sent, skillDoorLead), sent) + } + agent.chatlog.close() + stored, err := agent.config.Memory.Messages(agent.threadID(), 0, 0) + if err != nil { + t.Fatal(err) + } + found := false + for _, message := range stored { + if message.Role == "user" { + found = true + if message.Body != skillDoorWords { + t.Errorf("the store kept more than the person's words: %q", message.Body) + } + } + } + if !found { + t.Error("the store has no record of the person's message") + } + if err := agent.Close(); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(journal) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), skillDoorLead) { + t.Error("resubmitting the rewind draft journaled the injected block") + } +} + +// Compaction re-journals only the person's words on both ordinary and steering +// paths. A fresh agent draws those words while the live model copy keeps its block. +func TestCompactedConversationKeepsThePersonsWordsWhenSkillsRide(t *testing.T) { + for _, steered := range []bool{false, true} { + name := "ordinary message" + if steered { + name = "steering message" + } + t.Run(name, func(t *testing.T) { + journal := filepath.Join(t.TempDir(), "session.jsonl") + agent, _ := skillDoorAgent(t, journal) + agent.config.ContextWindow = 2000 + agent.contextWindow.Store(2000) + user := userText(skillDoorWords) + if steered { + user.crossed = &SteerMark{Consumed: true, Landing: "stopped the reply here"} + } + agent.mu.Lock() + agent.attachTurnSkillsLocked(&user) + agent.recordUserLocked(user) + if steered { + // The window's original message carries the correction mark. The + // projection must read it before replacing the content allocation. + agent.file.mu.Lock() + rememberSteer(agent.file.steers, user.message, *user.crossed) + agent.file.mu.Unlock() + } + agent.recordLocked(textMessage("assistant", strings.Repeat("thinking about the parser. ", 100)+"oldest sweep")) + agent.recordUserLocked(userText("the second question")) + agent.recordLocked(textMessage("assistant", strings.Repeat("thinking about the parser. ", 100)+"newer sweep")) + agent.recordUserLocked(userText("the third question")) + agent.recordLocked(textMessage("assistant", "the short last word")) + agent.mu.Unlock() + changed, err := agent.compact(context.Background(), nil) + if err != nil || !changed { + t.Fatalf("real compaction did not run: changed=%v err=%v", changed, err) + } + skillDoorDisplayed(t, agent.Transcript()) + found := false + for _, message := range agent.snapshot() { + if message.Role == "user" && strings.HasPrefix(messageContentText(message), skillDoorWords) { + found = true + if strings.Count(messageContentText(message), skillDoorLead) != 1 { + t.Fatal("compaction changed the live model copy's skills block") + } + } + } + if !found { + t.Fatal("the compaction window did not keep the skill-carrying message") + } + if err := agent.Close(); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(journal) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), skillDoorLead) { + t.Error("compaction journaled the model's skills block") + } + restored, _ := skillDoorAgent(t, journal) + skillDoorDisplayed(t, restored.Transcript()) + if steered { + for _, entry := range restored.Transcript() { + if entry.Text == skillDoorWords && (entry.Steer == nil || !entry.Steer.Consumed || entry.Steer.Landing != user.crossed.Landing) { + t.Error("compaction lost the original message's steering mark") + } + } + } + }) + } +} + +// Why quotes only the person's opening words while the model's copy keeps the +// skills block that informed the turn. +func TestWhyQuotesThePersonsWordsWhenSkillsRide(t *testing.T) { + agent, completer := skillDoorAgent(t, "", skillDoorReply) + skillDoorSubmit(t, agent, skillDoorWords) + if got := agent.Why(); !strings.HasPrefix(got, "> "+skillDoorWords+"\n\n") || strings.Contains(got, skillDoorLead) { + t.Errorf("Why quotes more than the person's words: %q", got) + } + if strings.Count(skillDoorRequest(t, completer, skillDoorWords), skillDoorLead) != 1 { + t.Fatal("the model's copy lost the skills block") + } +} + +// The conversation's title is drawn from the person's words, without skills +// context. The main model still receives its own copy with the block intact. +func TestTitleReadsThePersonsWordsWhenSkillsRide(t *testing.T) { + journal := filepath.Join(t.TempDir(), "session.jsonl") + agent, completer := skillDoorAgent(t, journal, skillDoorReply) + asks := make(chan string, 1) + completer.aside = func(messages []ai.Message) (*ai.Response, bool) { + if !isTitleCall(messages) { + return nil, false + } + select { + case asks <- userTextIn(messages): + default: + } + return textResponse("repository lint rules and checks"), true + } + skillDoorSubmit(t, agent, skillDoorWords) + select { + case input := <-asks: + if !strings.Contains(input, skillDoorWords) || strings.Contains(input, skillDoorLead) { + t.Errorf("the title request carries more than the person's words: %q", input) + } + case <-time.After(10 * time.Second): + t.Fatal("the title request never reached the provider") + } + if strings.Count(skillDoorRequest(t, completer, skillDoorWords), skillDoorLead) != 1 { + t.Fatal("the main model's copy lost the skills block") + } +} diff --git a/internal/session/title.go b/internal/session/title.go index 1ebd670a44..a6f2cf31b6 100644 --- a/internal/session/title.go +++ b/internal/session/title.go @@ -520,7 +520,7 @@ func (a *Agent) firstExchangeLocked() (string, string) { if question != "" && !shellOpening { return question, "" } - question = strings.TrimSpace(messageContentText(message)) + question = strings.TrimSpace(a.presentation.personWords(message)) case "assistant": // Human shell turns journal as user/call/result. Their durable call // mark, not a leading ! in ordinary model input, identifies them. diff --git a/internal/session/why.go b/internal/session/why.go index d03e569d79..b3c3e228ec 100644 --- a/internal/session/why.go +++ b/internal/session/why.go @@ -40,7 +40,7 @@ func (a *Agent) Why() string { return "Nothing to explain yet: this session has not been asked for anything." } - asked := clip(strings.TrimSpace(messageContentText(a.messages[start])), whyLimit) + asked := clip(strings.TrimSpace(a.presentation.personWords(a.messages[start])), whyLimit) turn := a.messages[start+1:] results := toolResults(turn)