Skip to content

fix(compress): Bug 39 — protected tool messages excluded from effectiveMessageIds - #32

Merged
ranxianglei merged 1 commit into
masterfrom
2026-08-02_bug39-effective-coverage
Aug 2, 2026
Merged

fix(compress): Bug 39 — protected tool messages excluded from effectiveMessageIds#32
ranxianglei merged 1 commit into
masterfrom
2026-08-02_bug39-effective-coverage

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Problem

Bug 39 (opencode-acp parity): protected tool messages (e.g. skill, compress) should be hard-excluded from compression. The README documented this as a "Known limitation" — but it was a real bug, not a deliberate simplification.

filterProtectedToolMessages (compress.ts:686) correctly drops protected tool calls and their paired results from directMessageIds. But the result was never written back to effectiveMessageIds. The new block therefore recorded protected messages as covered:

directMessageIds: filteredIds,                    // ← filtered ✓
effectiveMessageIds: [...effectiveMessageIds],    // ← UNFILTERED ✗ (bug)

Consequences:

  • Protected tool messages hidden from view (marked as covered by the block)
  • If later GC-truncated, the detail is unrecoverable
  • Decompress/search incorrectly treat them as compressed

Why it was invisible

Existing tests (protected-content.test.ts) only asserted on directMessageIds — never effectiveMessageIds. So the bug passed all 211 tests.

Fix

compress.ts: after filterProtectedToolMessages, delete removed ids from the effectiveMessageIds Set so both arrays agree:

if (filteredIds.length < directMessageIds.length) {
  const kept = new Set(filteredIds);
  for (const id of directMessageIds) {
    if (!kept.has(id)) effectiveMessageIds.delete(id);
  }
}

Tests

Added effectiveMessageIds assertions to the two skill-protection tests. Verified they fail without the fix (2 failures) and pass with it.

README

Replaced the "Known limitation" note — protected tool messages are now hard-excluded, matching opencode-acp Bug 39.

Test plan

  • 211 tests pass (was 211; 2 existing tests strengthened)
  • typecheck clean
  • New assertions fail without the fix, pass with it

…ds (Bug 39)

filterProtectedToolMessages correctly dropped protected tool calls (and
their paired results) from directMessageIds, but the result was never
written back to effectiveMessageIds. The new block therefore recorded
protected messages as covered, hiding them from view and folding them
into the block's effective coverage — exactly opencode-acp's Bug 39.

Existing tests only asserted on directMessageIds, so the bug was
invisible. The README even documented it as a 'Known limitation'.

Fix: after filtering, delete the removed ids from the effectiveMessageIds
Set so both arrays agree. Add effectiveMessageIds assertions to the two
skill-protection tests (verified they fail without the fix).

Update README: protected tool messages are now hard-excluded, matching
opencode-acp Bug 39. This is no longer a known limitation.
@ranxianglei
ranxianglei merged commit 4887b45 into master Aug 2, 2026
3 checks passed
@ranxianglei ranxianglei mentioned this pull request Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant