WIP

[nodo]

Note

Low Risk
Low risk because it only adds a new strategy-level test and test-only fake agent; production logic is unchanged.

Overview
Adds readonly_heuristic_test.go, a regression test that reproduces an edge case where shouldCondenseWithOverlapCheck can skip condensation for an ACTIVE session when another concurrent session claims overlapping FilesTouched but transcript-based filesTouchedBefore is empty.

The test introduces a test-only fakeAgentWithTranscriptAnalyzer to simulate “no new modifications since last condensation,” then asserts PostCommit still writes the checkpoint to entire/checkpoints/v1 (so entire explain can resolve the Entire-Checkpoint trailer) and updates the session’s LastCheckpointID.

^{Reviewed by Cursor Bugbot for commit b9f5775. Configure here.}

[Copilot]

Pull request overview

This PR adds a regression test in the manual-commit strategy area to cover a PostCommit edge case where an ACTIVE session can be incorrectly treated as “read-only” (and skipped for condensation) when another concurrent session’s FilesTouched overlaps the committed files.

Changes:

• Add a new strategy-level test (TestPostCommit_ReadOnlyHeuristic_BlocksActiveSession) that exercises PostCommit condensation behavior under concurrent-session overlap conditions.
• Introduce a local fake agent implementing transcript-analyzer capabilities to simulate “no new writes since last condensation offset”.

[Copilot]

GetTranscriptPosition is expected to return a transcript position (line count for JSONL or message count for JSON), but this stub returns len(transcript) in bytes. That can make hasNewTranscriptWork/offset comparisons behave incorrectly if the fake is reused in other tests; consider counting JSONL lines (or delegating to the real parser) instead.

[Copilot]

ExtractModifiedFilesFromOffset returns currentPosition=0 unconditionally. The TranscriptAnalyzer contract expects currentPosition to reflect the current transcript position (line/message count), and callers may persist that value as the next offset; returning 0 can cause repeated rescans or incorrect offset updates if this stub is reused elsewhere.

Suggested change

	return f.modifiedFiles, 0, nil
	return f.modifiedFiles, len(f.transcript), nil

[Copilot]

agent.Register factories are expected to create a new agent instance, but this closure always returns the same shared *fakeAgentWithTranscriptAnalyzer pointer. If any code calls agent.Get/GetByAgentType multiple times (or tests run in parallel), this can introduce shared mutable state and data races; return a fresh fake instance from the factory (or clone the struct) instead.

Suggested change

	fakeAgent := &fakeAgentWithTranscriptAnalyzer{
	fakeExternalAgent: fakeExternalAgent{name: agentBName, agentType: agentBType},
	transcript: transcriptContent,
	modifiedFiles: nil, // No new file modifications since last condensation offset
	}
	agent.Register(agentBName, func() agent.Agent { return fakeAgent })
	agent.Register(agentBName, func() agent.Agent {
	return &fakeAgentWithTranscriptAnalyzer{
	fakeExternalAgent: fakeExternalAgent{name: agentBName, agentType: agentBType},
	transcript: append([]byte(nil), transcriptContent...),
	modifiedFiles: nil, // No new file modifications since last condensation offset
	}
	})