fix(kernel): re-render Reference Knowledge + Your Team tails after TOML drift#3164
Merged
Conversation
…ML drift Closes #3143. #3142 fixed the `## User Configuration` tail loss; this picks up the two siblings the same drift block was clobbering — `## Reference Knowledge` (skill content from `def.skill_content` / `def.agent_skill_content`) and `## Your Team` (peer roster for multi-agent hands). Approach (option B from #3143): 1. New idempotent helpers `apply_skill_reference_block_to_manifest` and `apply_team_block_to_manifest` in `manifest_helpers.rs`, modelled on `apply_settings_block_to_manifest`. Both strip any pre-existing tail before re-appending so repeated calls (drift loop firing back-to-back) do not duplicate the section, and both gracefully degrade to "strip only" when the input collapses to empty (skill removed from hand / hand edited down to single agent) so stale tails are dropped instead of preserved indefinitely. 2. `manifest_for_diff()` projection: strips all three known runtime-rendered tails (settings, reference, team) before the JSON compare in the boot drift loop. Without it, the disk TOML always looks "different" from the DB blob on hand-with-tails agents and the drift branch fires every restart, burning a save_agent write and clobbering the rendered tails. With it, drift only fires when the *raw* TOML genuinely diverged from the *raw* DB form. 3. The activation site (`activate_hand_with_id`) is refactored to call both new helpers instead of inlining the rendering, and the drift block calls them after the existing settings re-render — keeping both paths through one source of truth, same pattern #3142 established for settings. Tests: 9 new unit tests covering empty-input no-op, idempotency, stale-tail replacement, and skill-removed tail-drop for the two new helpers; 3 manifest_for_diff tests (full / subset / no tails); 2 boot drift integration tests — `boot_drift_preserves_skill_and_team_tails` seeds a multi-agent hand with SKILL.md, forces drift via a body-text edit, asserts both new tails are re-rendered after reboot; `boot_drift_skipped_when_only_rendered_tails_differ` proves the sanitized projection prevents spurious drift fires. `cargo build --workspace --lib`, `cargo test --workspace`, and `cargo clippy --workspace --all-targets -- -D warnings` all clean locally (the 2 pre-existing `cron_delivery::webhook_*` failures from the loopback host policy are unchanged from main and unrelated).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Review fixes for #3164. - Fence `## Your Team` marker with `\n\n---\n\n` so a literal `## Your Team` line in SKILL.md or a base prompt cannot collide with the strip lookup. Keep the pre-fence form as a one-shot legacy fallback so agents persisted before this change migrate forward on the next drift cycle. - Add an ordering-contract note on both helpers so a future reorder of the activation chain (settings -> reference -> team) cannot silently strand a downstream tail. - Log at debug! when a hand-derived agent is missing its `hand_role:<role>` tag during drift; previously the skip was completely silent and the agent would degrade until the next manual `hand activate`. Tests: 24 manifest_helpers unit tests + 3 boot_drift integration tests pass; clippy clean.
…role drift path Review feedback on #3164: - Annotate LEGACY_TEAM_TAIL_MARKER with a SAFETY note about the substring collision with the fenced TEAM_TAIL_MARKER (callers must check the fenced form first), and a TODO marker for the eventual cleanup once every persisted prompt has been migrated through one drift cycle. - Add boot_drift_skips_tail_render_when_hand_role_tag_missing — proves the role lookup at kernel/mod.rs:~3057 is a guard rather than an assertion. When manifest.tags carries hand:<id> but is missing the hand_role:<role> companion, drift must apply the disk TOML body change without panicking and without re-rendering the role-keyed tails (since we cannot pick the right per-role override). The next hand activate re-stamps both tags and restores the tails.
3 tasks
houko
added a commit
that referenced
this pull request
Apr 25, 2026
* chore(kernel): drop LEGACY_TEAM_TAIL_MARKER fallback now that #3164 is merged The legacy unfenced `\n\n## Your Team` strip path was added in #3164 to migrate hand-derived agents persisted before the fence was introduced. Any DB blob carrying the unfenced form would be picked up by the boot drift loop on first boot post-#3164 and rewritten with the fenced form, so the fallback is single-use. User base is small enough that we don't need a multi-release migration window or telemetry confirmation — going straight to the cleanup. Removes: - LEGACY_TEAM_TAIL_MARKER constant + its SAFETY/TODO doc block - the `.or_else(... LEGACY_TEAM_TAIL_MARKER)` fallback in apply_team_block_to_manifest, leaving a plain fenced `find()` - the apply_team_block_migrates_legacy_unfenced_tail_in_place test (the migration code path no longer exists) Edge case (DB cross-version copy from pre-#3164 directly into a post-cleanup binary) is acceptable: worst outcome is a duplicated `## Your Team` block, self-healing on the next `hand activate` since the fenced strip then matches both copies. cargo test -p librefang-kernel --lib manifest_helpers # 24 passed cargo clippy -p librefang-kernel --tests --lib -- -D warnings # clean * test(kernel): lock down post-cleanup behavior for unfenced team tail Adds a regression test asserting that after dropping LEGACY_TEAM_TAIL_MARKER, a prompt carrying the pre-fence form is left intact and a fresh fenced block is appended alongside it (resulting in two team headings in the prompt). This is the deliberate trade-off documented in the cleanup PR — making it explicit so a future change that reintroduces unfenced detection trips this assertion before landing.
This was referenced Apr 25, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #3143. Follow-up to #3142 — extends the same drift-aware re-render
pattern to the two other runtime-rendered prompt tails the boot-time
TOML drift loop was clobbering.
What was still broken after #3142
#3142 fixed
## User Configuration. The drift block(
kernel/mod.rs:~2929) was still stripping two more rendered tailsappended by
activate_hand_with_id(~kernel/mod.rs:9131-9168) onevery restart:
## Reference Knowledge— skill content fromdef.skill_content(shared) or
def.agent_skill_content(per-role override). Lossmeant the agent dropped its skill discoverability summary; it
could still call
skill_read_file({skill, path})on demand but onlyif it knew what to ask for, so first-turn skill consideration was
silently degraded — most visible on
session_mode=newtriggers andcron fires.
## Your Team— peer roster for multi-agent hands. Loss meant theagent stopped knowing who its peers were and started calling
agent_listredundantly at the top of every session, plusagent_sendto peers became guesswork.Both were silent: no error, no warning, just degraded context.
Approach (option B from #3143)
New idempotent helpers in
manifest_helpers.rsmodelled onapply_settings_block_to_manifest:apply_skill_reference_block_to_manifest(manifest, role, def)— strips any pre-existing
## Reference Knowledgetail, thenre-appends from per-role override (
def.agent_skill_content)falling back to shared
def.skill_content. When neither is setthe call is strip-only — covers the case where a skill was
removed from the hand and the stale tail must drop.
apply_team_block_to_manifest(manifest, role, def)— stripsany pre-existing
## Your Teamtail, then re-appends the peerroster excluding self. No-op for single-agent hands; gracefully
drops the tail if a hand was edited from multi-agent down to
single-agent.
(
SKILL_REFERENCE_TAIL_MARKER,TEAM_TAIL_MARKER) foridempotency, same pattern as
USER_CONFIG_TAIL_MARKER.manifest_for_diff()projection strips all three knownruntime-rendered tails before the JSON compare in the boot drift
loop. Without it the disk TOML always looks "different" from the DB
blob on hand-with-tails agents and drift fires every restart,
burning a
save_agentwrite and clobbering the rendered tails.With it, drift only fires when the raw TOML genuinely diverged
from the raw DB form.
Activation site refactored to call both new helpers instead of
inlining the rendering. The drift block calls them after the
existing settings re-render. Both code paths now route through one
source of truth — same pattern fix(kernel): re-render hand [[settings]] tail after boot-time TOML drift #3142 established for settings.
Why approach B and not A
#3143 outlined two paths. A (per-tail helper, no projection) only
prevents tail loss on a legitimate drift fire; B adds the projection
on top, so a hand with rendered tails no longer triggers a spurious
drift fire on every boot. The class of bug recurs every time someone
adds a new rendered tail; the marker registry approach scales —
register the marker, the projection picks it up automatically.
Tests
14 new unit tests + 2 boot drift integration tests, all green:
apply_skill_reference_*— appends when present / no-op when empty /idempotent / replaces stale tail / drops tail when skill removed.
apply_team_block_*— no-op for single-agent / appends peersexcluding self / uses
invoke_hintover description / idempotent.manifest_for_diff_*— strips all known tails / strips any subset /no-op on prompts without tails.
boot_drift_preserves_skill_and_team_tails— multi-agent hand withSKILL.md, force drift via body-text edit, assert both tails are
re-rendered after reboot.
boot_drift_skipped_when_only_rendered_tails_differ— proves thesanitized projection prevents spurious drift fires when nothing
changed except the rendered tails (which are runtime-only and
expected to differ between disk TOML and DB blob).
cargo build --workspace --lib,cargo test --workspace, andcargo clippy --workspace --all-targets -- -D warningsall clean.Risk / blast radius
Low. Helpers are idempotent; the activation path's behaviour is
byte-identical to the old inline rendering on a fresh hand, and
distinguishable only on a re-render where the new path correctly
replaces a stale tail rather than letting it accumulate. Standalone
agents (no
hand:<id>tag, no role tag) skip the new code pathentirely.