diff --git a/devlog/2026-07-25_inactive-block-fixes/REQ.md b/devlog/2026-07-25_inactive-block-fixes/REQ.md new file mode 100644 index 00000000..3cffdd65 --- /dev/null +++ b/devlog/2026-07-25_inactive-block-fixes/REQ.md @@ -0,0 +1,30 @@ +# REQ: Inactive Block Fixes + +## Problem + +Two issues with inactive (consumed/GC'd/user-decompressed) compression blocks: + +1. **decompress rejects inactive blocks**: `resolveBlockTarget` in `lib/compress/decompress.ts` + rejects any block where `activeBlocks.length === 0` with "not active. It may have already been + decompressed." — even for standalone inactive blocks that are safe to decompress. The model sees + the block's compress call in context but cannot decompress it. + +2. **acp_status hides inactive blocks**: `scope:"compressed"` only iterates `activeBlockIds`, + completely hiding consumed/GC'd blocks. The model cannot discover blocks that were consumed by + secondary compression, making it impossible to reason about the full compression history. + +## Fix + +1. **decompress**: Remove the "not active" rejection in `resolveBlockTarget`. Keep the nested + redirect (consumed blocks inside an active parent still redirect to the parent). Standalone + inactive blocks (user-decompressed, GC'd, orphaned) can now be decompressed. + +2. **acp_status**: Show ALL blocks from `blocksById` (not just active ones). Add `[inactive]` + marker to the metadata line (not the topic line). Add "N active, M inactive/consumed" summary. + +## Files + +- `lib/compress/decompress.ts` — remove "not active" rejection +- `lib/compress/status.ts` — show all blocks, add inactive marker +- `tests/acp-status.test.ts` — 3 new tests (inactive visibility, user-decompressed, no marker on active) +- `tests/decompress-logic.test.ts` — 2 new tests (standalone inactive, consumed with active ancestor) diff --git a/devlog/2026-07-25_inactive-block-fixes/WORKLOG.md b/devlog/2026-07-25_inactive-block-fixes/WORKLOG.md new file mode 100644 index 00000000..5a348a9c --- /dev/null +++ b/devlog/2026-07-25_inactive-block-fixes/WORKLOG.md @@ -0,0 +1,57 @@ +# WORKLOG: Inactive Block Fixes + +## Changes + +### lib/compress/decompress.ts +- `resolveBlockTarget` (line ~120): Removed the `return { ok: false, error: "not active" }` branch. + Previously, when `activeBlocks.length === 0` and no active ancestor was found, decompress was + rejected. Now, standalone inactive blocks pass through to `{ ok: true, targets: [target] }`. + The nested-redirect (consumed blocks inside an active parent) is kept — decompressing a consumed + child directly would restore 0 messages since the parent still claims them. + +### lib/compress/status.ts +- `createAcpStatusTool` scope:"compressed" handler (line ~436): Changed block source from + `activeBlockIds` (active only) to `blocksById.values()` (all blocks). Inactive/consumed blocks + now appear in the compressed drilldown. +- `renderCompressedDrilldown` (line ~340): Added `[inactive]` marker to the metadata line for + blocks where `!b.active`. Added "N active, M inactive/consumed" summary line when inactive blocks + exist. Active blocks get no marker (default state). + +### tests/acp-status.test.ts +- 3 new tests: + - scope=compressed shows inactive/consumed blocks with marker and summary + - scope=compressed marks user-decompressed blocks as inactive + - scope=compressed does not add inactive marker to active blocks + +### tests/decompress-logic.test.ts +- 2 new tests: + - findActiveAncestorBlockId returns null for standalone inactive block (the condition that now + allows decompress to proceed) + - findActiveAncestorBlockId returns active ancestor for consumed inactive block (the condition + that still triggers the redirect) + +## Verification + +- typecheck: pass +- tests: 859 pass (846 existing + 13 new across 4 commits), 0 fail +- build: pass + +## Round 2 dual-agent review findings (fixed in commit 4) + +### Fixed: toFile writes garbage for inactive blocks +`lib/compress/decompress.ts:325` — inactive blocks had empty `activeBlocks`, so fallback was +`activeBlocks[0]?.summary` (undefined) → wrote literal `"(no content available)"`. Changed to +`targets[0]?.blocks[0]?.summary` so the block's actual summary is written. + +### Fixed: /acp decompress slash command still rejected inactive blocks +`lib/commands/decompress.ts:153-161` — same "not active" rejection existed in the slash command +path. Applied the same fix: keep nested-redirect, drop "not active" rejection. + +### Added: E2E tests for actual decompress tool behavior +`tests/inactive-block-decompress.test.ts` — 7 tests exercising the real decompress tool: +- resolveCompressionTarget returns target for inactive block +- decompress tool succeeds for standalone inactive block (actual behavior change) +- decompress tool redirects for consumed block with active parent +- decompress tool works normally for active block (control) +- toFile on inactive block writes summary (not placeholder) +- decompress succeeds when all ancestor chain is inactive (multi-block scenario) diff --git a/lib/commands/decompress.ts b/lib/commands/decompress.ts index 40dae1d2..b6fe0eb6 100644 --- a/lib/commands/decompress.ts +++ b/lib/commands/decompress.ts @@ -149,15 +149,6 @@ export async function handleDecompressCommand(ctx: DecompressCommandContext): Pr ) return } - - await sendIgnoredMessage( - client, - sessionId, - `Compression ${target.displayId} is not active.`, - params, - logger, - ) - return } const activeMessagesBefore = snapshotActiveMessages(messagesState) diff --git a/lib/compress/decompress.ts b/lib/compress/decompress.ts index 915db6ce..23d14727 100644 --- a/lib/compress/decompress.ts +++ b/lib/compress/decompress.ts @@ -117,6 +117,8 @@ function resolveSingleBlockTarget( } } + // Keep nested-redirect (consumed blocks must go through parent) but drop the + // "not active" rejection — inactive standalone blocks can be decompressed. const activeBlocks = target.blocks.filter((block) => block.active) if (activeBlocks.length === 0) { const activeAncestorBlockId = findActiveAncestorBlockId(messagesState, target) @@ -126,10 +128,6 @@ function resolveSingleBlockTarget( error: `Error: Block ${target.displayId} is nested inside active block ${activeAncestorBlockId}. Decompress block ${activeAncestorBlockId} first.`, } } - return { - ok: false, - error: `Error: Block ${target.displayId} is not active. It may have already been decompressed.`, - } } return { ok: true, targets: [target] } @@ -324,7 +322,7 @@ export function createDecompressTool(ctx: ToolContext): ReturnType const fileContent = lines.length > 0 ? lines.join("\n\n---\n\n") - : (activeBlocks[0]?.summary ?? "(no content available)") + : (targets[0]?.blocks[0]?.summary ?? "(no content available)") await writeFile(targetPath, fileContent, "utf-8") const displayIds = targets.map((t) => `b${t.displayId}`).join(", ") diff --git a/lib/compress/status.ts b/lib/compress/status.ts index 33c55bc0..a78279a5 100644 --- a/lib/compress/status.ts +++ b/lib/compress/status.ts @@ -338,6 +338,13 @@ function renderCompressedDrilldown( lines.push("") const shown = sorted.slice(0, limit) + const activeCount = sorted.filter((b) => b.active).length + const inactiveCount = sorted.length - activeCount + if (inactiveCount > 0) { + lines.push(`${activeCount} active, ${inactiveCount} inactive/consumed`) + lines.push("") + } + for (const b of shown) { const survived = b.survivedCount ?? 0 const gen = b.generation ?? "young" @@ -346,9 +353,10 @@ function renderCompressedDrilldown( b.consumedBlockIds && b.consumedBlockIds.length > 0 ? ` nested=[${b.consumedBlockIds.map((n) => `b${n}`).join(",")}]` : "" + const status = b.active ? "" : " [inactive]" const topic = b.topic || "(no topic)" lines.push( - ` b${b.blockId} ${formatTokens(b.compressedTokens)}→${formatTokens(b.summaryTokens)} ${formatAge(b.createdAt)} ${formatIdRange(b)} age=${survived} ${gen} eff=${effCount}${consumed}`, + ` b${b.blockId} ${formatTokens(b.compressedTokens)}→${formatTokens(b.summaryTokens)} ${formatAge(b.createdAt)} ${formatIdRange(b)} age=${survived} ${gen} eff=${effCount}${consumed}${status}`, ) lines.push(` "${topic}"`) } @@ -433,10 +441,9 @@ export function createAcpStatusTool(ctx: ToolContext): ReturnType { Number.isFinite(args.limit) && args.limit! > 0 ? Math.min(args.limit!, 200) : 30 const msgState = ctx.state.prune.messages - const activeIds = Array.from(msgState.activeBlockIds).sort((a, b) => a - b) - const allBlocks = activeIds - .map((id) => msgState.blocksById.get(id)) - .filter((b): b is NonNullable => b !== undefined && b.active) + const allBlocks = Array.from(msgState.blocksById.values()).sort( + (a, b) => a.blockId - b.blockId, + ) const lines: string[] = [] @@ -471,7 +478,7 @@ export function createAcpStatusTool(ctx: ToolContext): ReturnType { ...renderOverview( visibleMsgs, summaryTokens, - allBlocks, + allBlocks.filter((b) => b.active), fetchFailed, rawMessages, ctx, diff --git a/tests/acp-status.test.ts b/tests/acp-status.test.ts index 339f444e..6d5ca6b5 100644 --- a/tests/acp-status.test.ts +++ b/tests/acp-status.test.ts @@ -344,3 +344,71 @@ test("acp_status: invalid sort falls back to size", async () => { assert.match(result, /Sorted by size/) }) + +test("acp_status: scope=compressed shows inactive/consumed blocks", async () => { + const blocks = blocksMap( + makeBlock({ blockId: 1, active: true, compressedTokens: 5000, topic: "live" }), + makeBlock({ + blockId: 2, + active: false, + compressedTokens: 3000, + topic: "consumed", + parentBlockIds: [1], + }), + ) + const result = await runStatus([1], blocks, { scope: "compressed" }) + + assert.match(result, /b2/) + assert.match(result, /"consumed"/) + assert.match(result, /\[inactive\]/) + assert.match(result, /1 active, 1 inactive\/consumed/) +}) + +test("acp_status: scope=compressed marks user-decompressed blocks as inactive", async () => { + const blocks = blocksMap( + makeBlock({ + blockId: 5, + active: false, + deactivatedByUser: true, + topic: "user-decompressed", + }), + ) + const result = await runStatus([], blocks, { scope: "compressed" }) + + assert.match(result, /b5/) + assert.match(result, /\[inactive\]/) + assert.match(result, /0 active, 1 inactive\/consumed/) +}) + +test("acp_status: scope=compressed does not add inactive marker to active blocks", async () => { + const blocks = blocksMap(makeBlock({ blockId: 1, active: true, topic: "active block" })) + const result = await runStatus([1], blocks, { scope: "compressed" }) + + assert.match(result, /b1/) + assert.doesNotMatch(result, /\[inactive\]/) +}) + +test("acp_status: overview (no scope) excludes inactive blocks from count and totals", async () => { + const blocks = blocksMap( + makeBlock({ + blockId: 1, + active: true, + summaryTokens: 1000, + compressedTokens: 5000, + topic: "live", + }), + makeBlock({ + blockId: 2, + active: false, + summaryTokens: 500, + compressedTokens: 3000, + topic: "dead", + deactivatedByUser: true, + }), + ) + const result = await runStatus([1], blocks) + + assert.match(result, /1 active/) + assert.doesNotMatch(result, /2 active/) + assert.doesNotMatch(result, /"dead"/) +}) diff --git a/tests/decompress-logic.test.ts b/tests/decompress-logic.test.ts index 2c45a0db..f67adb66 100644 --- a/tests/decompress-logic.test.ts +++ b/tests/decompress-logic.test.ts @@ -580,3 +580,25 @@ test("resolveDecompressMode: empty string startId/endId → error (treated as mi assert.equal(result.ok, false) if (!result.ok) assert.match(result.error, /Must specify either/) }) + +// --- Inactive block decompress --- + +test("findActiveAncestorBlockId returns null for standalone inactive block", () => { + const inactiveBlock = makeBlock({ blockId: 5, active: false, parentBlockIds: [] }) + const ms = makeMessagesState({ blocksById: new Map([[5, inactiveBlock]]) }) + const target = makeTarget({ blocks: [inactiveBlock] }) + assert.equal(findActiveAncestorBlockId(ms, target), null) +}) + +test("findActiveAncestorBlockId returns active ancestor for consumed inactive block", () => { + const activeParent = makeBlock({ blockId: 10, active: true }) + const consumedBlock = makeBlock({ blockId: 5, active: false, parentBlockIds: [10] }) + const ms = makeMessagesState({ + blocksById: new Map([ + [5, consumedBlock], + [10, activeParent], + ]), + }) + const target = makeTarget({ blocks: [consumedBlock] }) + assert.equal(findActiveAncestorBlockId(ms, target), 10) +}) diff --git a/tests/inactive-block-decompress.test.ts b/tests/inactive-block-decompress.test.ts new file mode 100644 index 00000000..086e7856 --- /dev/null +++ b/tests/inactive-block-decompress.test.ts @@ -0,0 +1,251 @@ +import assert from "node:assert/strict" +import test from "node:test" +import { createDecompressTool } from "../lib/compress/decompress" +import type { ToolContext } from "../lib/compress/types" +import type { + CompressionBlock, + PrunedMessageEntry, + SessionState, + WithParts, +} from "../lib/state/types" +import { resolveCompressionTarget } from "../lib/commands/compression-targets" +import { findActiveAncestorBlockId } from "../lib/compress/decompress-logic" + +const SID = "session-inactive-decompress-e2e" + +function makeBlock(overrides: Partial = {}): CompressionBlock { + return { + blockId: 1, + runId: 1, + active: true, + deactivatedByUser: false, + compressedTokens: 1000, + summaryTokens: 200, + durationMs: 0, + mode: "range", + topic: "test", + batchTopic: "test", + startId: "m00001", + endId: "m00003", + anchorMessageId: "anchor-1", + compressMessageId: "comp-1", + compressCallId: undefined, + includedBlockIds: [], + consumedBlockIds: [], + parentBlockIds: [], + directMessageIds: ["msg-a", "msg-b"], + directToolIds: [], + effectiveMessageIds: ["msg-a", "msg-b"], + effectiveToolIds: [], + createdAt: 1000, + deactivatedAt: undefined, + deactivatedByBlockId: undefined, + summary: "Compressed conversation about topic X.", + survivedCount: 0, + generation: "young", + ...overrides, + } +} + +function makeState(blocks: CompressionBlock[], activeIds: number[]): SessionState { + const blocksById = new Map() + for (const b of blocks) { + blocksById.set(b.blockId, b) + } + return { + sessionId: SID, + isSubAgent: false, + manualMode: false, + compressPermission: "allow", + pendingManualTrigger: null, + prune: { + tools: new Map(), + messages: { + byMessageId: new Map(), + blocksById, + activeBlockIds: new Set(activeIds), + activeByAnchorMessageId: new Map(), + nextBlockId: blocks.length + 1, + nextRunId: blocks.length + 1, + markedForCleanup: new Set(), + }, + }, + nudges: { + contextLimitAnchors: new Set(), + turnNudgeAnchors: new Set(), + iterationNudgeAnchors: new Set(), + lastPerMessageNudgeTurn: 0, + lastPerMessageNudgeTokens: undefined, + }, + stats: { pruneTokenCounter: 0, totalPruneTokens: 0 }, + compressionTiming: {} as any, + toolParameters: new Map(), + subAgentResultCache: new Map(), + toolIdList: [], + messageIds: { byRawId: new Map(), byRef: new Map(), nextRef: 1 }, + lastCompaction: 0, + currentTurn: 0, + modelContextLimit: 100000, + systemPromptTokens: undefined, + } +} + +function makeToolContext(state: SessionState): ToolContext { + const noop = () => {} + return { + client: { + session: { + messages: async () => ({ data: [] as WithParts[] }), + }, + } as any, + state, + logger: { + enabled: false, + info: noop, + warn: noop, + error: noop, + debug: noop, + } as any, + config: { manualMode: { enabled: false } } as any, + prompts: { reload: () => {} } as any, + } +} + +function makeRunContext(): { ask: any; metadata: any; sessionID: string } { + return { + ask: async () => {}, + metadata: () => {}, + sessionID: SID, + } +} + +async function runDecompress( + state: SessionState, + args: Record, +): Promise { + const ctx = makeToolContext(state) + const tool = createDecompressTool(ctx) + return tool.execute(args as any, makeRunContext() as any) +} + +// --- resolveCompressionTarget returns target for inactive blocks --- + +test("resolveCompressionTarget returns target for inactive standalone block", () => { + const block = makeBlock({ blockId: 5, active: false, deactivatedByUser: false }) + const state = makeState([block], []) + const target = resolveCompressionTarget(state.prune.messages, 5) + assert.ok(target !== null) + assert.equal(target!.displayId, 5) + assert.equal(target!.blocks.length, 1) +}) + +test("resolveCompressionTarget returns null for non-existent block", () => { + const state = makeState([], []) + const target = resolveCompressionTarget(state.prune.messages, 99) + assert.equal(target, null) +}) + +// --- E2E: decompress tool with standalone inactive block --- + +test("E2E: decompress tool succeeds for standalone inactive block", async () => { + const inactiveBlock = makeBlock({ + blockId: 5, + active: false, + deactivatedByUser: false, + parentBlockIds: [], + }) + const state = makeState([inactiveBlock], []) + + const result = await runDecompress(state, { blockId: "b5" }) + + assert.ok(!result.includes("not active"), `should not reject: ${result}`) + assert.ok(!result.includes("Error"), `should not error: ${result}`) + assert.match(result, /Decompressed/) +}) + +test("E2E: decompress tool redirects for consumed block with active parent", async () => { + const activeParent = makeBlock({ blockId: 10, active: true }) + const consumedBlock = makeBlock({ + blockId: 5, + active: false, + parentBlockIds: [10], + }) + const state = makeState([activeParent, consumedBlock], [10]) + + const result = await runDecompress(state, { blockId: "b5" }) + + assert.match(result, /nested inside active block/) + assert.match(result, /Decompress block 10 first/) +}) + +test("E2E: decompress tool works normally for active block", async () => { + const activeBlock = makeBlock({ blockId: 1, active: true }) + const state = makeState([activeBlock], [1]) + + const result = await runDecompress(state, { blockId: "b1" }) + + assert.ok(!result.includes("Error"), `should not error: ${result}`) + assert.match(result, /Decompressed/) +}) + +// --- E2E: toFile on inactive block writes summary, not placeholder --- + +test("E2E: toFile on inactive block writes block summary", async () => { + const inactiveBlock = makeBlock({ + blockId: 5, + active: false, + summary: "Important compressed content about feature X.", + }) + const state = makeState([inactiveBlock], []) + + const result = await runDecompress(state, { + blockId: "b5", + toFile: "/tmp/test-inactive-block-decompress.txt", + }) + + assert.ok(!result.includes("Error"), `should not error: ${result}`) + assert.match(result, /written to/) + assert.ok( + !result.includes("(no content available)"), + `should not write placeholder: ${result}`, + ) + + const { readFileSync } = await import("fs") + const fileContent = readFileSync("/tmp/test-inactive-block-decompress.txt", "utf-8") + assert.equal(fileContent, "Important compressed content about feature X.") +}) + +// --- E2E: multi-block scenario (consumed chain) --- + +test("E2E: decompress succeeds when all ancestor chain is inactive", async () => { + const grandchild = makeBlock({ + blockId: 3, + active: false, + parentBlockIds: [2], + summary: "Innermost block summary.", + }) + const child = makeBlock({ + blockId: 2, + active: false, + parentBlockIds: [1], + consumedBlockIds: [3], + summary: "Middle block summary.", + }) + const parent = makeBlock({ + blockId: 1, + active: false, + consumedBlockIds: [2], + summary: "Outermost block summary.", + }) + const state = makeState([parent, child, grandchild], []) + + const ancestorId = findActiveAncestorBlockId(state.prune.messages, { + displayId: 3, + blocks: [grandchild], + } as any) + assert.equal(ancestorId, null, "no active ancestor in fully-inactive chain") + + const result = await runDecompress(state, { blockId: "b3" }) + assert.ok(!result.includes("Error"), `should not error: ${result}`) + assert.match(result, /Decompressed/) +})