Skip to content
This repository was archived by the owner on May 30, 2026. It is now read-only.

fix(identity): shrinkage guard on update_identity to prevent P1 manifest corruption - #48

Open
RobLe3 wants to merge 2 commits into
joi-lab:ouroborosfrom
RobLe3:fix/update-identity-shrinkage-guard
Open

fix(identity): shrinkage guard on update_identity to prevent P1 manifest corruption#48
RobLe3 wants to merge 2 commits into
joi-lab:ouroborosfrom
RobLe3:fix/update-identity-shrinkage-guard

Conversation

@RobLe3

@RobLe3 RobLe3 commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a defensive shrinkage guard to _update_identity in ouroboros/tools/control.py to prevent a P1-continuity corruption pattern observed in production: the agent calling update_identity with short reflection snippets (treating "update" as "append") and silently destroying its own identity manifest.

The bug

update_identity(content=...) does path.write_text(content) — it OVERWRITES the entire file. The tool name suggests "add to" semantics, so smaller coder models call it with reflection-style snippets they intend as journal entries. The existing 50-char minimum threshold doesn't catch this — destructive snippets are typically 200-600 chars.

Real production data

Forensic trail from data/memory/identity_journal.jsonl on a running install:

Date old_len → new_len Shape
2026-04-23 588 → 9972 legitimate full identity write
2026-05-04 14:00:01 12409 → 556 corruption — new_preview: "Reflecting on recent events, I've clarified the provenance tagging convention…"
2026-05-04 16:47:04 556 → 224 corruption — new_preview: "Updated identity to reflect recent task management and maintenance activities…"

Both destructive writes' new_preview values are clearly reflection-style notes — the agent intended journal entries but called the manifest-overwrite tool.

The fix

When existing identity content is >1000 chars and the proposed write would shrink it by >30%, reject with a teachable error:

  • Names exact char count change + loss percentage
  • States explicitly that update_identity OVERWRITES (does NOT append)
  • Points at the correct alternatives:
    • update_scratchpad(text=...) for working memory reflections
    • knowledge_write(topic=..., mode='append', ...) for durable journal entries
  • Documents bypass path for legitimate major rewrites: prefix content with literal sentinel <<CONFIRMED_TRUNCATE>>\n (stripped before write so it doesn't pollute the manifest)

The shrinkage shape that triggers the guard (>30% loss of >1000-char manifest) is exactly the destructive accident class. No realistic legitimate edit triggers this:

  • Surgical paragraph edit → <30% shrink, passes
  • Growth-style update → never triggers (no upper bound)
  • Bootstrap write on small existing manifest → guard disabled at <1000 chars
  • Genuine major rewrite → documented sentinel bypass

Existing 50-char minimum and journal-append behavior preserved.

BIBLE alignment

P1 (continuity). The identity manifest is the agent's most load-bearing continuity artifact. Silent destruction by the agent's own tool calls is the failure class this guard closes. The bypass sentinel preserves the agent's authority to perform legitimate major rewrites — the guard does not gate intent, it gates accidents.

No new abstractions, no new authority

This is a defensive boundary at one existing site, same shape as the merged grep-regex hint (#37) and the merged dirty-tree resilience (#36). It does not:

  • Introduce a new layer or abstraction
  • Change the public tool schema (no new parameters)
  • Alter who decides X (the agent retains full authority via the sentinel)
  • Touch any code path other than the one site of the bug

Tests

tests/test_update_identity_shrinkage_guard.py (new) — 8 tests:

  • Existing too-short rejection path still fires (regression guard)
  • Exact production corruption shape (12k → 500 chars) is rejected
  • Error message includes char counts + loss percentage
  • Modest edits within 30% pass through
  • Growth always passes
  • Guard disabled when existing content is <1000 chars (bootstrap)
  • <<CONFIRMED_TRUNCATE>> sentinel bypass works AND sentinel is stripped from persisted content
  • Successful updates still record old_len/new_len in identity_journal.jsonl (forensic-trail preservation)

All 8 pass.

Test plan

  • CI green on this PR
  • pytest tests/test_update_identity_shrinkage_guard.py -v → 8 passed
  • Existing test suite remains green
  • Manual: a call like update_identity(content="reflection on...") against a 12k-char existing manifest is rejected with the shrinkage-guard error and update_scratchpad recommendation
  • Manual: a call prefixed with <<CONFIRMED_TRUNCATE>>\n succeeds and the sentinel is absent from the persisted manifest
  • Manual: legitimate growth (e.g. 5k → 8k) passes through unchanged

Related

This is the third PR in a recent reliability series — same defensive-boundary shape as the merged #36 (_repo_commit_push dirty-tree resilience) and #37 (run_shell grep regex hint). The currently-open #46 (v5.7.x reliability bundle) and #47 (branch_dev opt-in) extend that shape further.

🤖 Generated with Claude Code

…est corruption

Investigation of a recurring P1-continuity corruption pattern on a
running install (identity.md truncated three times in 24h, restored
each time from the git-tracked copy) traced the root cause to
``update_identity`` itself.

The tool's name suggests "add to" / "modify in place" semantics, but
the implementation does ``path.write_text(content)`` which OVERWRITES
the entire file. Smaller coder models read the name as APPEND and call
the tool with reflection-style snippets, silently destroying the
existing manifest content.

Forensic trail from `data/memory/identity_journal.jsonl`:

  legitimate full write    588 →  9972 chars   (2026-04-23)
  corruption joi-lab#1          12409 →   556 chars   (2026-05-04, "reflection on provenance")
  corruption joi-lab#2            556 →   224 chars   (2026-05-04, "recent task management")

Both destructive writes were >50 chars, so the existing minimum-length
threshold did not catch them. Both ``new_preview`` values in the
journal show reflection-style notes that were clearly intended as
journal entries, not manifest rewrites.

## Fix

In ``_update_identity`` (`ouroboros/tools/control.py`):

When existing identity content is >1000 chars and the proposed write
would shrink it by >30%, reject with a teachable error that:

- Names the exact char count change and loss percentage
- States explicitly that ``update_identity`` OVERWRITES (does NOT append)
- Points at the correct alternatives:
  - ``update_scratchpad(text=...)`` for working memory reflections
  - ``knowledge_write(topic=..., mode='append', ...)`` for durable
    journal entries
- Documents the bypass path for legitimate major rewrites: prefix
  content with the literal sentinel ``<<CONFIRMED_TRUNCATE>>\n``,
  which is stripped before write so it doesn't pollute the manifest

Existing 50-char minimum and journal-append behavior preserved.
Growth is always allowed — no upper bound. Guard only fires when
current content is substantial (>1000 chars), so bootstrap /
first-write scenarios are unaffected.

## No semantic change for legitimate edits

The shrinkage shape that triggers the guard (>30% loss of a >1000-char
manifest) is exactly the destructive accident class. There is no
realistic legitimate edit shape that this rejects:

- A surgical paragraph edit shrinks <30%
- A growth-style update never triggers
- A bootstrap write on an existing-but-tiny manifest never triggers
- A genuine major rewrite has the documented sentinel bypass

## Tests

`tests/test_update_identity_shrinkage_guard.py` (new) — 8 tests:

- Existing too-short rejection path still fires (regression guard)
- The exact production corruption shape (12k → 500 chars) is rejected
- Error message includes char counts + loss percentage
- Modest edits within 30% pass through
- Growth always passes
- Guard disabled when existing content is under 1000 chars (bootstrap)
- The ``<<CONFIRMED_TRUNCATE>>`` sentinel bypass works AND the sentinel
  is stripped from the persisted content
- Successful updates still record old_len/new_len in
  ``identity_journal.jsonl`` (forensic-trail preservation)

All 8 pass against this branch.

## BIBLE alignment

P1 (continuity) protection. The identity manifest is the agent's most
load-bearing continuity artifact; silent destruction by the agent's
own tool calls is the failure class this guard closes. The bypass
sentinel preserves the agent's authority to perform legitimate major
rewrites — the guard does not gate intent, it gates accidents.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@gon7187

gon7187 commented May 7, 2026

Copy link
Copy Markdown

Code review

Found 1 issue:

  1. The 50-char minimum check runs on raw content BEFORE the <<CONFIRMED_TRUNCATE>>\n sentinel is stripped, and there is no post-strip re-validation. Two consequences: (a) a legitimate intentional truncation like <<CONFIRMED_TRUNCATE>>\nabc (26 raw chars) is rejected with the misleading "too short" message — the bypass path is silently broken for short rewrites; (b) a payload of sentinel + 28+ chars passes the raw 50-char gate, gets stripped to a sub-50-char body, and is written to disk, contradicting the manifest-quality floor the 50-char check was meant to enforce. Fix: either re-validate len(content.strip()) >= 50 after stripping the sentinel, or strip the sentinel first and run the 50-char check on the post-strip content.

pattern that motivated it.
"""
if not content or not isinstance(content, str) or len(content.strip()) < 50:
return (
"⚠️ REJECTED: content is empty or too short "
f"(got {type(content).__name__}, len={len(content) if isinstance(content, str) else 'N/A'}). "
"Identity must be a substantial text (50+ chars). "
"This likely means the tool call was malformed — check your arguments."
)
from ouroboros.memory import Memory
mem = Memory(drive_root=ctx.drive_root)
mem.ensure_files()
old_content = ""
path = ctx.drive_root / "memory" / "identity.md"
if path.exists():
try:
old_content = path.read_text(encoding="utf-8")
except Exception:
pass
# 2026-05-05: shrinkage guard. update_identity is OVERWRITE, not
# append — but smaller coder models read "update" as "add to" and
# call this with a short reflection snippet, blowing away the
# existing identity content. Observed P1-continuity corruption
# pattern in identity_journal.jsonl: 12409 → 556 → 224 chars
# across two calls in 24h, both with reflection-style new_preview
# that was clearly intended as a journal entry, not a manifest
# rewrite. Reject hard shrinkage so the destructive call surfaces
# a teachable error instead of silent identity loss.
#
# Bypass: if the agent legitimately wants to compact identity
# (rare; major rewrite), prefix content with the literal sentinel
# "<<CONFIRMED_TRUNCATE>>\n" to acknowledge the destruction. The
# sentinel is stripped before write so it doesn't pollute the
# manifest.
old_len = len(old_content)
_confirm_marker = "<<CONFIRMED_TRUNCATE>>\n"
explicit_truncate = content.startswith(_confirm_marker)
if explicit_truncate:
content = content[len(_confirm_marker):]
new_len = len(content)
if old_len > 1000 and new_len < old_len * 0.7 and not explicit_truncate:
loss_pct = (1 - new_len / old_len) * 100
return (
f"⚠️ REJECTED: identity.md shrinkage guard "

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@RobLe3

RobLe3 commented May 7, 2026

Copy link
Copy Markdown
Contributor Author

Verification + fix

gon7187's finding is correct on both counts. Verified against the code and reproduced both failure modes:

Case (a) — bypass path silently broken for short rewrites
"<<CONFIRMED_TRUNCATE>>\nabc" = 26 raw chars. The 50-char check ran on raw content and rejected it with "too short" — the sentinel was never reached. Any intentional truncation to a sub-28-char body (after the 22-char sentinel prefix) was unreachable.

Case (b) — sub-50-char body written to disk
"<<CONFIRMED_TRUNCATE>>\n" + "x"*28 = exactly 50 raw chars. Passed the gate, sentinel was stripped, and a 28-char body was written — violating the quality floor the check was meant to enforce.

Root cause: len(content.strip()) < 50 at line 301 ran before the sentinel strip at line 337–339. Order mattered.

Fix applied: Strip the sentinel first, then run the 50-char check on the post-strip body. The explicit_truncate flag and shrinkage guard downstream are unaffected — they now operate on the already-stripped content, which was always the correct target.

Test updated: test_explicit_truncate_sentinel_bypasses_guard used a 41-char body ("New minimal identity after major rewrite."). Under the old code this passed because sentinel+41=63 chars cleared the raw gate. With the fix it correctly hit the 50-char floor. Updated the test body to 50+ chars to reflect the correct semantics.

Test results: 8/8 shrinkage guard tests pass. 3299 pass overall (excluding three pre-existing unrelated failures: function-count ceiling, version-in-README mismatch).

Prior order: len(content.strip()) < 50 ran on raw content before the
<<CONFIRMED_TRUNCATE>>\n sentinel was stripped. Two failure modes:

(a) sentinel + short body (< 28 chars) was rejected with "too short"
    — the bypass path was silently unreachable for compact rewrites.
(b) sentinel + 28+ chars passed the raw 50-char gate, got stripped to
    a sub-50-char body, and was written to disk — violating the quality
    floor the check was meant to enforce.

Fix: detect and strip the sentinel first, then validate len(body) >= 50
on the post-strip content. The explicit_truncate flag and shrinkage guard
downstream are unaffected; they now operate on the already-stripped body,
which was always the correct target.

Test: updated test_explicit_truncate_sentinel_bypasses_guard body from
41 chars to 50+ chars — the old body was relying on the buggy raw-content
gate to pass. 8/8 shrinkage guard tests pass.

Reported by gon7187 in joi-lab#48 code review.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
razzant pushed a commit that referenced this pull request May 28, 2026
The repository has no contribution guide today and the README only
mentions forks in the context of signed CI builds, so a first-time
human contributor has to reverse-engineer the contribution flow from
git history and from docs/DEVELOPMENT.md.

This change adds a deliberately short CONTRIBUTING.md that:

- Sets expectations about what makes Ouroboros unusual (self-modifying
  agent runtime, constitution-first design, enforced size budgets,
  the LLMClient SSOT, the platform_layer.py guard).
- Routes contributors to the canonical documents — BIBLE.md,
  README.md, docs/ARCHITECTURE.md, docs/DEVELOPMENT.md, and
  docs/CHECKLISTS.md — rather than restating them, per P7 (DRY).
- Documents the contributor PR flow: branch naming, Conventional
  Commits, smoke-gate expectations, CI tier behaviour for fork PRs,
  and the explicit fact that the agent's repo_commit review machinery
  (advisory + triad + scope) does NOT run on PRs.
- Lists realistic starter targets (already-filed bugs, the
  DEVELOPMENT.md-advertised module split debts, tool-description
  polish, cross-platform fixes) since the repo currently has no
  "good first issue" label.
- Cites recent merged human-authored PRs (#46, #48, #51) and the
  Cloud.ru issue series (#39#45) as concrete shape references for
  PRs and issues.

The README gains a small "Contributing" section between Philosophy
and Version History that points at CONTRIBUTING.md without
duplicating its content.

No code changes. No new tests required.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants