Skip to content

fix(kernel): re-render Reference Knowledge + Your Team tails after TOML drift#3164

Merged
houko merged 4 commits into
mainfrom
fix/hand-rendered-tails-drift
Apr 25, 2026
Merged

fix(kernel): re-render Reference Knowledge + Your Team tails after TOML drift#3164
houko merged 4 commits into
mainfrom
fix/hand-rendered-tails-drift

Conversation

@houko

@houko houko commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

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 tails
appended by activate_hand_with_id (~kernel/mod.rs:9131-9168) on
every restart:

  • ## Reference Knowledge — skill content from def.skill_content
    (shared) or def.agent_skill_content (per-role override). Loss
    meant the agent dropped its skill discoverability summary; it
    could still call skill_read_file({skill, path}) on demand but only
    if it knew what to ask for, so first-turn skill consideration was
    silently degraded — most visible on session_mode=new triggers and
    cron fires.
  • ## Your Team — peer roster for multi-agent hands. Loss meant the
    agent stopped knowing who its peers were and started calling
    agent_list redundantly at the top of every session, plus
    agent_send to peers became guesswork.

Both were silent: no error, no warning, just degraded context.

Approach (option B from #3143)

  1. New idempotent helpers in manifest_helpers.rs modelled on
    apply_settings_block_to_manifest:

    • apply_skill_reference_block_to_manifest(manifest, role, def)
      — strips any pre-existing ## Reference Knowledge tail, then
      re-appends from per-role override (def.agent_skill_content)
      falling back to shared def.skill_content. When neither is set
      the 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) — strips
      any pre-existing ## Your Team tail, then re-appends the peer
      roster excluding self. No-op for single-agent hands; gracefully
      drops the tail if a hand was edited from multi-agent down to
      single-agent.
    • Both use marker-anchored truncation
      (SKILL_REFERENCE_TAIL_MARKER, TEAM_TAIL_MARKER) for
      idempotency, same pattern as USER_CONFIG_TAIL_MARKER.
  2. manifest_for_diff() projection strips all three known
    runtime-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_agent write and clobbering the rendered tails.
    With it, drift only fires when the raw TOML genuinely diverged
    from the raw DB form.

  3. 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 peers
    excluding self / uses invoke_hint over 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 with
    SKILL.md, force drift via body-text edit, assert both tails are
    re-rendered after reboot.
  • boot_drift_skipped_when_only_rendered_tails_differ — proves the
    sanitized 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 test -p librefang-kernel --lib
# 26 new tests pass; the 9 pre-existing failures
# (config::tests::test_load_config_defaults + 8 cron_delivery::*)
# are unchanged from main and unrelated to this PR.

cargo build --workspace --lib, cargo test --workspace, and
cargo clippy --workspace --all-targets -- -D warnings all 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 path
entirely.

…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).
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot added ready-for-review PR is ready for maintainer review area/kernel Core kernel (scheduling, RBAC, workflows) labels Apr 25, 2026
houko added 3 commits April 26, 2026 00:50
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.
@houko
houko merged commit e9b4331 into main Apr 25, 2026
13 checks passed
@houko
houko deleted the fix/hand-rendered-tails-drift branch April 25, 2026 16:00
@github-actions github-actions Bot added size/L 250-999 lines changed and removed ready-for-review PR is ready for maintainer review labels Apr 25, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kernel Core kernel (scheduling, RBAC, workflows) size/L 250-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Boot-time TOML drift clobbers runtime-rendered prompt tails (Reference Knowledge, Your Team) — settings tail fixed in #3142, others still broken

1 participant