host: one over-long entry in a memory review answer discards the whole answer and the review never moves past those lines #2558

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead
Claimed-by: FlyBot Lead
Claim-Id: b5e766031cde4e9abf02c8fd9dc595d4
Branch: dev/5-memory-review-unfit-entry
Base: c907f46
At: 2026-11-06T01:09:23Z

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment.

Written-by: FlyBot Lead

What happens today

core/src/host/memory.js (main 49fda03f) parses a memory-review answer into changes, then vets the entries before it applies anything:

• :1486-1487 — const unfit = applied.filter(...).find(text => !secretEntry(text) && entryProblem(text) !== undefined), and then if (unfit !== undefined) throw new Error('the model's answer holds an entry memory cannot keep: …').
• That throw runs before applyReview (:1492), so every other change in the same answer is dropped with it — a legal remove, edit or add never lands. The answer is all-or-nothing over a single over-long entry.
• The cursor is saved only on the success path (:1506). queueReview's catch (:1538-1556) only writes one rt.warn line, so the next review reads exactly the same lines again — there is no cap on that path (REVIEW_PASSES_MAX at :1497 caps only the overflow re-reads, which is a different branch).
• The same file already handles a single unfit entry correctly inside applyReview (:1393: if ((op === 'add' || op === 'edit') && entryProblem(text) !== undefined) continue), so "leave out that one entry" is the existing semantics for a change set; the pre-check that throws for the whole answer is the outlier.

Host diagnostics show this firing in production, on consecutive reviews of the same session, with entries of 303, 561, 308 and 358 characters — each one threw away its whole answer and left the cursor where it was.

Expected

• One entry that memory cannot keep is left out on its own, with one warning, and the rest of the answer applies (a remove in the same answer must land; an over-long add must not).
• The cursor moves past the lines that review read, so the next review does not read the same user lines again.
• An answer that genuinely cannot land keeps failing as it does today: a malformed answer, an answer naming an entry that is not listed, and a full MEMORY.md are unchanged.
• An entry that is or contains a secret keeps the current silent drop (secretEntry), and remove is never filtered.

Reproduction (red first)

On a clean 49fda03f, with only the new test file change applied, core/test/memory.test.js fails:

not ok 1 - an entry memory cannot keep is left out, the rest of the answer lands, and the review moves on
  location: 'core/test/memory.test.js:786:1'
  error: The input was expected to not match the regular expression /Likes tea/.
    '# Memory\n\n- Likes tea [source: part 1 #10; added: …]\n\n## Index\n'
  code: 'ERR_ASSERTION'  operator: 'doesNotMatch'
# tests 1 / # pass 0 / # fail 1

The test remembers Likes tea, then hands the review an answer holding one 348-character add plus a legal {op: 'remove', entry: 1}. On today's main the whole answer is dropped, so Likes tea is still in MEMORY.md.

Readings from the same probe, base vs fixed:

• base: {"requestsAfterFirst":1,"requestsAfterSecond":2,"secondPromptHoldsSameUserLine":true,"cursor":{"chat:b2":9},"memoryHoldsTea":true,"warnedFailed":2} — a second review is scheduled and its prompt still carries the same user line.
• fixed: {"requestsAfterFirst":1,"requestsAfterSecond":1,"secondPromptHoldsSameUserLine":null,"cursor":{"chat:b2":11},"memoryHoldsTea":false,"warnedCannotKeep":1,"warnedFailed":0} — cursor 9 → 11, the remove landed, the 348-character entry is not in MEMORY.md, and exactly one warning of the new kind.

Scope

• core/src/host/memory.js — replace the find + throw with a filter (problemOf returns undefined for remove, secrets stay out silently), warn once for the first entry left out, and hand the surviving changes to applyReview.
• core/test/memory.test.js — the new case above. The existing case at :762 asserts the old all-or-nothing semantics for a too-long entry, so its too-long leg moves into the new case; the malformed / unlisted / bad-topic / password legs stay where they are. No assertion is loosened.
• patches/ds-bot/0001-core-fixes.patch — regenerated (generate, then canonical back to a no-op). memory.test.js is inside the ds-bot + patches check, so this is required.
• Not in scope: core/src/host/text.js and the ends() / reviewLine() lines (the lone-surrogate work in host: the memory review's short user line and Bot line keep an unpaired surrogate in the prompt #2556/host: the memory review's short user line and Bot line keep an unpaired surrogate in the prompt #2557 runs there); the badTopic throw right below this one (same family, separate slice); a backoff for reviews that keep failing for other reasons (separate candidate).

Gates (from the local READY on this base)

• TZ=UTC ARCH_DOCS_BASE=49fda03f npm run verify → exit 0, core 2944/2944, arch-docs: OK (3 changed file(s))
• node scripts/ds-bot-patches.mjs canonical → ok patches/ds-bot/ is what generate writes (generate is a no-op)
• core/test/memory.test.js → 99/99
• numstat vs 49fda03f: memory.js +11/−5, memory.test.js +31/−4, patches/ds-bot/0001-core-fixes.patch +82/−2

Dedupe

Checked before opening: no open or closed issue covers this surface — searches for the warning text, "memory review discards", "unfit entry" and the review cursor return nothing on it. The nearest neighbours are different work: #485 (Reflexion, learn from repeated failures as a feature), #1918 (the prompt cutting on UTF-16 units), #2556/#2557 (unpaired surrogates in the review lines), and #1886/#1895/#1898 (code points in other memory paths). Product's audit put this on the backlog as P-600.

Risk tier: T2 (core/src/host/**).