fix(publishers): fix BufferedPublisher retains its buffer after close

[LizzyC-115]

Summary

In the original implementation, BufferedPublisher would close by calling publish_batch() then close(). However, publish_batch() did not clear the buffer and when close() is called twice, a ValueError occurs.

This PR introduces a _closed flag in the init of the BufferedPublisher. This flag is used to check the closed state of the publisher. If we are closed, then we will avoid touching the buffer, making close() be idempotent. Additionally, we add a self.buffer.close() call to clear the buffer when close() is called so the buffer ends in an empty state.

Type of change

• Bug fix (fix)
• New feature (feat)
• Breaking change (feat! / fix!)
• Refactor (refactor)
• Documentation (docs)
• Chore / tooling (chore)

Verification

Tests

• Unit tests added or updated
• Existing tests cover this change
• No tests — explain why:

Checklist

• PR title follows Conventional Commits (e.g. feat(driver): add support for Keysight E36300)
• I have read CONTRIBUTING.md
• Documentation updated if user-facing behavior changed
• Code follows the style/conventions of the surrounding code

Notes for reviewers

Adding a logger warning when attempting to call close() or publish() after the BufferedPublisher is closed to keep consistency with the other publishers.

[LizzyC-115]

@claude review

[claude]

Claude finished @LizzyC-115's task in 2m 48s —— View job

---

Review: fix(publishers) double-close on BufferedPublisher

• Read trigger comment and gather context
• Review diff against AGENTS.md conventions
• Post findings

Overview: The fix is correctly targeted — _closed flag plus clearing self.buffer after flush in close() addresses the actual bug (stale buffer replayed into an already-closed sink on double-close). Good test coverage: idempotent-close, dropped-publish-after-close, and a real regression test against FilePublisher/jsonl that reproduces the original ValueError. Matches the existing QueuedPublisher/SharedPublisher pattern of warning-and-dropping on post-close calls, so it's consistent with the rest of the module. No new abstractions introduced — appropriately minimal.

Left 3 inline comments (all minor, non-blocking):

• publisher.py:45 — warning hardcodes "BufferedPublisher" instead of self.__class__.__name__; misreports subclass name.
• publisher.py:49 — close() now skips publish_batch() entirely when the buffer is empty (previously unconditional); harmless for the only current subclass, but a behavior change on a public abc.ABC contract worth a second look.
• tests/lib/test_buffered_publisher.py:20-25 — StrictRecordingPublisher docstring claims to isolate race conditions, but no test drives concurrent access, and the threading.Lock that was on BufferedPublisher itself in an earlier commit (3cb4c20) was dropped from the final diff — the docstring's framing is stale/overstated.

No driver-convention, docs-sync, or breaking-change concerns — this is an internal bugfix to existing lib code, not a public API change.

[greptile-apps]

PR author is not in the allowed authors list.

[claude]

Claude finished @LizzyC-115's task in 2m 59s —— View job

---

Review: fix(publishers) double-close on BufferedPublisher

• Read trigger comment and gather context
• Re-review current state of the branch (post earlier round of feedback)
• Post findings

Overview: The three earlier findings from this branch's first review pass are all resolved-by-response: the hardcoded "BufferedPublisher" string was kept for consistency with QueuedPublisher's existing convention, the close()-skips-publish_batch()-on-empty-buffer behavior change was reverted (9598ccf), and the StrictRecordingPublisher concurrency docstring was explicitly scoped out. Current diff is small and correctly targeted: _closed flag + clearing self.buffer after the flush in close() fixes the real bug (double-close replaying a drained-but-not-cleared buffer into an already-closed sink), and the FilePublisher/jsonl regression test reproduces the original ValueError end-to-end. No new abstractions, no public API/signature change, no docs-sync trigger — appropriately scoped as an internal fix.

One new inline note: the test file pairs a weak (RecordingPublisher) and strict (StrictRecordingPublisher) sink for both the repeated-close and publish-after-close invariants, plus the real-FilePublisher regression test — three ways of asserting largely the same thing. Left a comment on test_repeated_close_never_touches_underlying_publisher_twice suggesting trimming the strict-sink variants since a call counter already fully proves "not called twice," per AGENTS.md's guidance against redundant test matrices.

No further action needed from me unless you want the redundant tests trimmed.