add Poolside Agent CLI pool agent support - #1241
Conversation
roborev: Combined Review (
|
- termination.go: remove blanket ResultEvents skip that defeated running-only event detection, regressing TerminationToolCallPending - poolside_provider.go: use streaming discovery instead of os.ReadDir to propagate I/O errors instead of silently returning empty results - poolside.go: increase scanner buffer to 10MB for large NDJSON records and use io.Copy for fingerprinting instead of line-limited scanner - poolside.go: track firstEventTime from first valid timestamp instead of firstInferenceTime for correct StartedAt computation
roborev: Combined Review (
|
- peak context now includes cache_read and cache_write tokens for accurate session peak context computation - session.error events now set exit reason to trigger truncated termination status instead of incorrectly classifying as clean - malformed NDJSON lines are counted in MalformedLines field - replaced bufio.Scanner with newLineReader using maxLineSize (64MB) to handle large tool-result observations without failing entire sessions
roborev: Combined Review (
|
The model attribution in assistant_message.end was nested inside AssistantMessage != "", so tool-only assistant turns (no text content) lost their model. Extract the attribution to run whenever the last message is an assistant, regardless of content emptiness. Add TestParsePoolsideSessionToolOnlyModelAttribution to cover the empty-content tool-calling case.
roborev: Combined Review (
|
Poolside's configured root is the entire application-data directory, but only trajectories/ is parsed. Provider-authoritative file-backed roots are recursively archived during remote sync, so the broad root could transfer unrelated configuration, caches, or credentials. Add resolvePoolsideTarget that resolves the root to only the trajectories/ subdirectory. Add matching av_emit_poolside_target in the SSH resolve script. Both HTTP and SSH resolvers now skip Poolside roots without an existing trajectories/ directory. Add parity tests in both remotesync and ssh packages to verify the narrowing behavior and the skip-when-missing path.
roborev: Combined Review (
|
Remote sync passes the trajectories/ subdirectory as Poolside's provider root, but poolsideDiscoverEach always appended trajectories/ to the root, resolving to trajectories/trajectories/ and discovering no files. Add poolsideTrajectoriesDir helper that returns the root as-is when its base component is already 'trajectories', otherwise appends trajectories/. Refactor poolsideDiscoverEach, poolsideWatchRoots, poolsideClassifyPath, and poolsideFindFile to use it. Add TestPoolsideTrajectoriesDir unit tests covering both root forms. Extend the SSH/HTTP parity test to include Poolside and verify both resolvers narrow to the trajectories/ subdirectory.
roborev: Combined Review (
|
…alization Two medium-severity issues from review: 1. classifyPoolsideTermination checked hasOrphanedToolCall before exit_tool_called, misclassifying normal Poolside exits (where the exit tool has no result event because the session terminates) as tool_call_pending. Check exit_tool_called first so trailing exit tools are classified as clean. 2. Both remote-sync resolvers always appended trajectories/ even when the configured root was already a trajectories directory, producing trajectories/trajectories and omitting sessions. Normalize direct trajectories roots to use them as-is in both HTTP and SSH resolvers. Add regression tests for both fixes.
roborev: Combined Review (
|
Three correctness issues from review: 1. thought.end overwrote ThinkingText instead of appending. Multiple reasoning blocks in one assistant turn now concatenate with newline, matching qwen/kimi/zcode/posit parsers. 2. Malformed final NDJSON records were counted but never set IsTruncated or TerminationStatus. A trajectory cut off mid-write now correctly sets IsTruncated and returns TerminationTruncated, taking precedence over other termination signals. 3. TestPoolsideTrajectoriesDir used hardcoded Unix paths that failed on Windows. Use filepath.FromSlash in assertions. Add Windows skip to TestResolveScriptPoolsideTrajectoriesRoot (POSIX shell script test, same pattern as existing skips).
roborev: Combined Review (
|
pendingToolCalls was keyed by step_id, so multiple tool_call.parsed events sharing the same step_id overwrote each other. Results were then attached to only the last parsed call, discarding the rest. Key pendingToolCalls by the payload call ID (shared between parsed and result events) so each call pairs with its own result independently. Add TestParsePoolsideMultipleCallsSameStep covering two reads in the same step with distinct results.
roborev: Combined Review (
|
Two medium-severity issues from review: 1. poolsideFingerprintSource hashed the entire trajectory on every sync, making background work scale with total archive size. Change to stat-only (size/mtime), deferring the expensive content hash to parse time. The engine's skip cache uses MTimeNS for freshness and already handles empty hashes gracefully. 2. pendingShellCmds was keyed by step_id, so multiple shell calls sharing a step overwrote each other. Subsequent shell_status, shell_tail, or shell_kill calls could then be enriched with the wrong command. Key by payload call ID instead and delete after use. Add TestParsePoolsideMultipleShellCallsSameStep covering two shell calls in the same step with distinct shell_ids and commands.
roborev: Combined Review (
|
The evidence entry claimed no public source exists, but the pool release repository (github.com/poolsideai/pool) and the Poolside Agent CLI documentation (docs.poolside.ai/cli/pool) are both public and were linked from the pull request itself. Upstream confirms automatic session saving and per-session trajectory files, though it publishes neither the on-disk paths nor the NDJSON event schema, so the entry is now evidence-tier documentation with the schema still characterized from real trajectory files.
roborev: Combined Review (
|
Poolside support in #1241 registered three platform-specific default directories behind the single `POOLSIDE_DIR` override. The SSH resolver evaluates that override while iterating each default, so a non-empty override emits the same `trajectories` target three times. The Linux fallback correction in #1258 changed one default path but did not cause or resolve this duplication. Deduplicate resolved directory targets per agent before archive construction, preserving first-seen order and allowing different agents to share a path independently. Strengthen the Poolside override regression to require one parsed transfer target. This addresses the post-merge roborev finding on #1241. Co-authored-by: Matthew Jacobs <mjacobs@users.noreply.github.com>
2 participants
This PR adds agent support for the Poolside Agent CLI
poolTested on MacOS and Linux.