fix(agent): summary and compaction history correctness

[adriannoes]

Summary

Consolidates two cursor high-severity agent fixes (originally cursor/high-severity-issues-5d10 and cursor/high-severity-issues-104f) as operator-authored commits.

• fix(agent): include the compaction boundary message in the summary (supersedes fix(agent): include the compaction boundary message in the summary #102) — the kept tail starts at max_ctx-1, but the summary window stopped one message earlier, so that turn was deleted from the session and never summarized.
• fix(agent): keep history that does not fit the summary (supersedes fix(agent): keep history that does not fit the summary #108) — compaction deleted every message before the kept tail even when the summary source filled up first; a later short line replaced the truncated long line, so that history was removed without being summarized.

Testing

• make test_agent — passed
• CI=true make test — all 45 test binaries passed
• make static — passed

Supersedes #102 and #108.

[cursor]

Code review — PR #121

Repo context: ShellClaw is a C99 edge AI agent (Jetson / Raspberry Pi) with a ReAct loop in src/core, providers, tools, channels, gateway, ASAP, sandbox, and hardware. Compaction lives in compact_session_via_llm and is meant to shrink SQLite session history when msg_count > max_context_messages.

What this PR delivers: Two history-loss fixes in the compaction path (superseding #102 and #108). (1) The summary window now includes index msg_count - max_ctx so the turn at the keep-tail boundary is not deleted without being summarized. (2) When a message does not fit in SUMMARY_SOURCE_MAX, compaction stops and keeps that message and everything after it instead of skipping the long line and deleting it anyway.

Scope: src/core/agent.c, tests/test_agent.c only (+135/−10).

Verification: Local CI=true make test_agent → test_agent: all tests passed. GitHub static, test, release was still pending at review time.

---

Summary

Solid, focused correctness fix. The off-by-one math matches keep = max_ctx - 1 (one summary + tail), summary_source_append is atomic (whole line or roll back), and both regressions are covered by tests. No merge blockers in the claimed #102/#108 fixes. Remaining notes are follow-up debt from the prefer-keep policy and changelog hygiene.

Must Fix

None.

Should Fix

1. Oversized-message compaction stall (src/core/agent.c ~132–140, ~169–171) — If one message cannot fit in SUMMARY_SOURCE_MAX (64 KiB):

   • First message too large → used == 0 → return -1 → session unchanged → every later agent_run retries and fails (agent_prepare_context ignores the return code).
   • Later message too large → partial compact keeps from that index, so new_count stays > max_ctx. Later turns only re-summarize what sits before the blocker (often just the prior summary) and never reach max_ctx. Session growth can eventually hit SESSION_JSON_MAX / SESSION_LOAD_TOO_LARGE.
     Prefer a truncate-into-summary stub (or drop-with-marker) for the single oversized line so compaction can make progress. Rare under the 128 KiB session cap, but sticky once it happens, and each turn still pays a summarize LLM call.

2. CHANGELOG — Add Unreleased Fixed bullets for both history-loss fixes (#102 / #108). Same gap called out on recent shellclaw fix PRs (#119, #120).

Nice to Have

1. Stale comment in tests/test_agent.c (~388): it still says msg_10 was skipped by both windows; after the fix it is intentionally in the summary source.
2. Fit test only asserts marker + summary string; could also assert alpha_small left the raw history and that a follow-up turn still cannot shrink past the 65 KiB line (documents the stall).
3. next_unsummarized == 0 is redundant with used == 0 on the first-append failure path.

Positive Highlights

1. Boundary math is correct — With 50 messages and max_ctx = 40, summarize 0..10, keep from 11; test locks msg_10 in the summary source and msg_11 in the post-compact chat spy.
2. Atomic append + early break — Fixes the old “skip long line, accept a later short line, delete the long one” hole without mid-line truncation into the summarizer input.
3. Fail closed when nothing fits — Returning -1 and leaving the session alone is the right call versus silent deletion.

Dimension notes

Area	Notes
Correctness	Claimed fixes hold; edge case is the oversized-message stall above.
Architecture	Stays inside existing compact_session_via_llm; no layering issues.
Security	No new trust boundaries; summarizer still sees session text already stored for the agent.
Performance	Partial-compact path can re-invoke the summarizer every turn without shrinking (see Should Fix #1).
Tests	Both supersede bugs have regression coverage; make test_agent green locally.
Maintainability	Helper + comments are clear; CHANGELOG gap is the main polish miss.

  

_{Sent by Cursor Automation: Code review}

[cursor]

Should Fix: When next_unsummarized < to_summarize_n because a single line exceeds SUMMARY_SOURCE_MAX, start = next_unsummarized leaves summary + (msg_count - start) always above max_ctx. Later runs only re-summarize what sits before that blocker (often the previous summary) and never shrink the session. If the first message alone does not fit, the path returns -1 at ~137 and every agent_run retries with no progress (agent_prepare_context ignores the rc).

Prefer truncating that one oversized line into the summary source (with a clear omitted-bytes marker) or replacing it with a stub so compaction can advance. Prefer-keep over silent delete is right for #108; unbounded stall is the leftover debt.

[cursor]

Nice to Have: This comment still says msg_10 was skipped by both the summary window and the kept tail. After the to_summarize_n = msg_count - max_ctx + 1 fix, msg_10 is intentionally in the summary source (that is what compaction_source_has_boundary asserts). Suggest rewording to “boundary index that the old window omitted.”

[adriannoes]

Review response — all items addressed.

Must Fix: None raised.

Should Fix

1. Oversized-message compaction stall — Fixed in 2f3b505 (fix(agent): keep compaction progressing when one message cannot fit the summary). A single line that cannot fit even a fresh 64 KiB summary source is now summarized as a truncated stub ending in a [truncated] marker, so the pass consumes it instead of aborting with used == 0 (oversized first line) or keeping new_count > max_ctx forever (oversized later line). Lines that fit a fresh source but lack room in the current pass keep the prefer-keep path, and whole-line append-or-rollback atomicity for fitting lines is unchanged.
2. CHANGELOG — Fixed in 77fc20b (docs(changelog): record compaction history-loss, fold, and no-stall fixes). Unreleased/Fixed now covers the boundary-turn summary inclusion, history that cannot fit the summary being kept, the deferred summary save with the roles-alternating fold (superseding fix(agent): keep history when compaction cannot finish the turn #126), and the no-stall guarantee.

Nice to Have

1. Stale comment on the boundary index — Fixed in 2f3b505; the comment now states that msg_10 is the boundary turn the summary window includes, with the kept tail starting at msg_11.
2. Fit test assertions — Addressed in 2f3b505. The suggested "follow-up turn cannot shrink past the 65 KiB line" note is superseded by the stall fix itself; the fit test was reworked to pin the remaining prefer-keep behavior (a line that fits a fresh source but lacks room stays raw, fitting predecessor lines are asserted consumed), and the two new stall tests assert a follow-up turn shrinks the session back to max_context_messages.
3. Redundant next_unsummarized == 0 — Fixed in 2f3b505; the abort guard is now used == 0 only, keeping the fail-closed behavior for an all-malformed summary window.

Consolidation: Bugbot PR #126 (fix(agent): keep history when compaction cannot finish the turn) is consolidated into this branch in 3d048ed and has been closed as superseded.

Validation: make test_agent green — both stall tests fail on the parent commit (first-line case: "compaction aborted before the summarizer call"; later-line case: no [truncated] stub reached the summary) and pass on the fix. CI=true make test: every module suite green except the pre-existing macOS-only test_allowlist.c:694 failure, reproduced on a pristine development checkout (module untouched by this PR; GitHub CI runs Ubuntu). make static clean.