Move "clean" root command to "session cleanup", some refactoring by Soph · Pull Request #2 · entireio/cli

Soph · 2026-01-05T13:51:52Z

To have less commands on the first level of the CLI this moves "clean" under "session" and rename it to "cleanup".

Also allow each strategy to handle it's own cleanup (so far it only cleaned up shadow branches from the manual-commit strategy) and cleanup stale sessions and checkpoints too.

Entire-Checkpoint: 94de498254e2

Entire-Checkpoint: 9878b713483d

- make sure we compare the right parts of commit hashes - make sure we don't delete fresh created session states - log what gets cleaned up Entire-Checkpoint: 9878b713483d

Three follow-ups from review of this PR: 1. Bump the bufio.Scanner max line length from 16MB to 64MB in both parsers. Codex packs the entire stdout of command_execution tools into the aggregated_output field on item.completed envelopes inline, so a chatty grep/cat/find over a moderately-sized repo can put many MB into one envelope and hit the prior cap — surfacing as "review failed" with no clue what tipped it over. Claude shares the cap for parity; one buffer per active review, so memory cost is modest. Comment on both sites explains the choice. 2. Default arm on codex's envelope-type switch logs unknown types at Debug. The parser today handles thread.started, turn.started, item.started, item.completed, turn.completed; anything else falls through silently. If codex evolves (new tool item types, intermediate envelope variants), drift is now triageable via ENTIRE_LOG_LEVEL=debug instead of needing source-dive. Adds an explicit no-op case for thread.started / turn.started so they remain documented swallows rather than absorbed by the default. 3. Doc-comment polish on both parser functions: - "Exposed for golden-file contract testing" → "Package-private; called directly from this package's tests" (the parsers are lowercase / unexported, "Exposed" misread on first glance). - Add a line noting Tokens are emitted only at the terminal envelope, not incrementally — the PR's headline "live events" benefit applies to AssistantText/ToolCall, not Tokens, and readers expecting per-message token streaming would be surprised. Note on review feedback #2 (missing codex garbled-line test): turned out to be incorrect — TestParseCodexOutput_GarbledLineEmitsRunError AndContinues already exists at codex/reviewer_test.go:346. Symmetry is intact; no test added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 3dd5f29e5289

* feat(review): stream claude-code via --output-format stream-json claude -p plain-text mode buffers stdout until the model has finished generating, so the review drill-in (Ctrl+O) stayed at [started] for the whole run and the dashboard preview only populated at end. Switching to stream-json --verbose emits one JSON envelope per agent event (assistant messages, tool use, result), giving the parser per-message granularity and making Ctrl+O surface live progress. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: e3d911c520d2 * feat(review): stream codex via exec --json codex exec plain-text mode buffers chrome + assistant output until the model finishes, so the review drill-in stayed at [started] for codex agents. Switching to exec --json emits one JSON envelope per agent-side event (item.started for tool calls, item.completed for agent messages, turn.completed for the terminal usage block), giving the parser per-event granularity. The old chrome-filter state machine is no longer needed and has been removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: e2625d3943af * fix(review): post-loop Finished emit + streaming-before-EOF guard Address code-review feedback on the codex --json parser: - Match claudecode's post-loop Tokens+Finished emission pattern instead of emitting inline inside the turn.completed branch. Prevents double emit if codex ever sends multiple turn.completed envelopes and keeps the two agent parsers structurally parallel. - Re-add TestParseCodexOutput_StreamsEventsBeforeEOF (deleted in the chunk 2 rewrite) using NDJSON envelopes through io.Pipe. Guards against future regressions that re-introduce batching. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 6118267e4e52 * docs: drop chrome-filter references after codex --json switch The codex chrome filter (output_filter.go) was removed in f0b6eca / a3d29fe when codex switched to exec --json. Two CLAUDE.md prose lines still mentioned it: the reviewer.go inventory said "codex with chrome filter", and the anti-features list still referred to filterCodexOutput specifically. Tidy both to reflect the post-switch shape, with the anti-feature now stated as a general principle (per-agent parsers own their format; shared code only sees Event variants) so the design boundary is still documented. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: fb4673c45488 * fix(review): emit stream-json result in integration-test fake agent TestReviewCommand_PassesReviewEnvToSpawnedAgentHook uses a fake claude shell script whose stdout was empty (the script pipes its content to a hook's stdin). With chunk 1's switch to --output-format stream-json the new parser interpreted the empty stream as "no result envelope" and classified the agent run as Failed, breaking the test. Append a minimal valid stream-json result envelope to the fake script's stdout so the parser sees a clean session end. The hook-firing path the test actually exercises is unchanged. Apply the same fix to TestReviewCommandSmoke_IncludesCheckpointContextInPrompt whose stub printed plain text 'smoke review ok' that the new parser treated as malformed JSON. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 6066c1f3cd20 * chore(review): drop stale Strip references in codex scanner-error test The comments referred to Strip's internal scanner, which lived in output_filter.go and was deleted when codex switched to exec --json. parseCodexOutput now reads io.Reader directly with its own 16MB scanner buffer; the comments now reflect that. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 7df8d0da6d9e * test(review): cover streaming, missing-terminal, and garbled-line paths Address Tier A items from PR #1192 review: - Add TestParseClaudeOutput_StreamsEventsBeforeEOF mirroring codex's io.Pipe + timeout-gated streaming guard. Also exercises the previously-uncovered tool_use content block path. - Add TestParse{Claude,Codex}Output_NoTerminalEnvelopeMeansFailed for the truncated-stream → Finished{Success:false} branch. - Add TestParse{Claude,Codex}Output_GarbledLineEmitsRunErrorAndContinues to lock the recover-and-continue contract for per-line JSON errors. - Document codex's hard-coded Finished{Success:true} so a future schema addition that exposes turn-level errors gets wired through. - Soften the "codex 0.130.0" version-pin comment to read as a contract. - Replace the stale "no chrome filtering needed" comment in geminicli/reviewer.go with a description that doesn't reference deleted code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 4ab5685eff94 * fix(review): address PR bot feedback - Reuse messageUsage (claudecode/types.go) for the stream-json result envelope's Usage field instead of a duplicate claudeUsage struct, so the two consumers of Claude's API usage shape stay in lockstep (bugbot feedback on PR #1192). - Streaming-before-EOF tests for both agents: capture the expected event into a separate variable before type-asserting, so a failure message reflects the actual event type rather than the zero-valued asserted type (Copilot feedback on PR #1192). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: f516fe93f468 * review: widen scanner buffer + log unknown codex envelope types Three follow-ups from review of this PR: 1. Bump the bufio.Scanner max line length from 16MB to 64MB in both parsers. Codex packs the entire stdout of command_execution tools into the aggregated_output field on item.completed envelopes inline, so a chatty grep/cat/find over a moderately-sized repo can put many MB into one envelope and hit the prior cap — surfacing as "review failed" with no clue what tipped it over. Claude shares the cap for parity; one buffer per active review, so memory cost is modest. Comment on both sites explains the choice. 2. Default arm on codex's envelope-type switch logs unknown types at Debug. The parser today handles thread.started, turn.started, item.started, item.completed, turn.completed; anything else falls through silently. If codex evolves (new tool item types, intermediate envelope variants), drift is now triageable via ENTIRE_LOG_LEVEL=debug instead of needing source-dive. Adds an explicit no-op case for thread.started / turn.started so they remain documented swallows rather than absorbed by the default. 3. Doc-comment polish on both parser functions: - "Exposed for golden-file contract testing" → "Package-private; called directly from this package's tests" (the parsers are lowercase / unexported, "Exposed" misread on first glance). - Add a line noting Tokens are emitted only at the terminal envelope, not incrementally — the PR's headline "live events" benefit applies to AssistantText/ToolCall, not Tokens, and readers expecting per-message token streaming would be surprised. Note on review feedback #2 (missing codex garbled-line test): turned out to be incorrect — TestParseCodexOutput_GarbledLineEmitsRunError AndContinues already exists at codex/reviewer_test.go:346. Symmetry is intact; no test added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 3dd5f29e5289 * review: parameterize parser scanner cap so scanner-error tests hit the right branch The scanner-error tests in claudecode/reviewer_test.go and codex/reviewer_test.go wrote 17×1MB to trigger bufio.Scanner's "token too long" error, with comments referencing the previous 16MB cap. Commit bfc7556 widened the cap to 64MB but didn't touch the tests, so 17MB now fits in one token, json.Unmarshal fails on the huge blob, and the test passes through the unmarshal-error path without ever exercising scanner.Err() != nil. Tests passed for the wrong reason. Extract parseClaudeOutputBuf(r, maxBuf) / parseCodexOutputBuf(r, maxBuf) as package-private test seams; the public parseClaudeOutput / parseCodexOutput delegate with the production 64MB cap (now named constants claudeReviewMaxScannerBuf / codexReviewMaxScannerBuf with the rationale moved to the constant doc comment). Production behavior unchanged — same cap, same buffer, same code path. The two scanner-error tests now call the *Buf variant with a 4KB cap and an 8KB payload, which actually trips bufio.ErrTooLong. Assertion strengthened to require a RunError whose message starts with "read stdout:" (the scanner branch's prefix) rather than accepting any RunError, so future cap-widening or payload-shrinking will fail the test loudly instead of silently routing through the unmarshal path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: drop stale chrome-stripping reference in types/reviewer.go doc The package preamble still listed "codex's chrome stripping" as a per-agent quirk encapsulated behind AgentReviewer. With codex on `exec --json -` there is no chrome to strip — replace with codex's actual remaining quirk (its JSONL envelope shape). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

CI lint: - escape U+200B literals (staticcheck ST1018) Real bugs (cursor[bot] + Copilot review): - loop: ctx-cancel mid-cmd.Run() now classifies as OutcomeCancelled instead of being counted as a turn failure and tripping OutcomePaused after two consecutive cancels (#1) - loop: save state before returning OutcomePaused so --continue resumes from a snapshot that includes the failing turn (#11) - investigate fix: wrap context.Canceled as SilentError so Ctrl+C during the fix session doesn't print a cobra usage banner (#2) - cmd: redact URL userinfo on the issue-link Source: line in both interactive and non-interactive paths (#5) - issuelink: redact URL userinfo across gh stderr (not just argv) so a token in --issue-link cannot leak via the error path (#10) - cmd: outcome-aware footer — "Investigation ended" + resume hint for paused/cancelled, "Investigation complete" + fix hint only for Quorum/Stalled (#6) - cmd: validate maxTurns/quorum bounds after settings/flag merge so a hand-edited negative max_turns or oversized quorum errors cleanly instead of silently stalling (#7) - issuelink: tolerate GitHub URL trailing segments (/pull/123/files, trailing slash) — the regex now anchors prefix and ignores tail (#13) - picker: don't print "Saved investigate config" before persistence; moved to the caller after SaveLocal succeeds (#14) - picker: guard pickerFormOverride with atomic.Pointer so parallel tests that swap the override don't race (#12) Docs: - settings/loop: fix stale "0 → 3" max_turns doc; default is 2 (#8/#9) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: b0135abeee0d

…ewire mirror remove entiredb#1820 (accurate success codes) and the ENT-741 mirror refactor are now on main, so refresh the spec and simplify against it: - normalize.go: the success-code bug is fixed upstream (creates declare 200, deletes 204), so stop collapsing success to a "2XX" range. Keep the real success code and fold only the explicit 4xx/5xx into a single "default" (all reference one ErrorModel) — still convenient errors, but ogen now returns the success type directly with no *…StatusCode wrapper. collapseTypeUnions stays: #1 (nullable-array shorthand) is still unfixed. - Drop the .Response unwraps across org/project/repo/grant/repo_mirror now that methods return (*T, error); DELETEs return plain error. - mirror remove: the by-mirror lookup endpoint is gone; call the new delete-by-coords route directly (DELETE /mirrors?provider&owner&repo& clusterHost). 404 is now a real, surfaced error, not idempotent success. - UPSTREAM.md: reframe #2 (success-code bug fixed; the error-fold that remains is a deliberate ergonomics choice retired by a shared upstream "default"); remove #3 (resolved by delete-by-coords). Net ~2.4k fewer generated lines. fmt/lint/test:ci green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: e7de894317fb

Resolves the review packet on the Codex subagent change: - #1 Codex task-checkpoint UUID: documented that CheckpointUUID is Claude-format-specific (tool_result/UUID) and resolves to "" for Codex — rewind to a Codex task checkpoint restores files via the shadow tree but does not truncate the transcript (graceful fall-through; restore truncation is also Claude-only). Codex-aware line-offset truncation is a deferred follow-up. Comment at the call site + AGENT.md. - #2 Resumed-subagent token under-count: documented deliberate trade-off (chosen over the worse cross-turn double-count) + observability — logResumedOutOfRangeChildren debug-logs when a resume_agent targets a child spawned in a prior range. - #3 Regression tests: CalculateTotalTokenUsage nil-main-with-subagent-tokens unit test (guards the nil-deref); new integration test TestCodexSubagentEnd_SourcesFilesFromChildNotParent (+ SimulateCodexSubagentStart/Stop harness helpers) guarding handleLifecycleSubagentEnd's child-not-parent resolver. - #4 Debug logging across the discover→resolve→read→parse chain (readSubagentRolloutsUncached, CalculateTotalTokenUsage, spawn-output drop) so Codex wire-format drift is visible instead of silently zeroing attribution. - #5 isCodexSubagentRollout: nested source.subagent fallback for rollouts without thread_source + backward-compat test (missing marker → treated as parent turn). - #6 + nits: t.Parallel() on the pure-logic codex tests; AGENT.md PostToolUse line corrected (it IS consumed); StepTranscriptStart aligned to the resolved transcriptOffset (removes the exec/no-preState divergence; field is currently unconsumed by the strategy). go build (incl. integration tag), vet, unit + codex integration tests, lint (0) all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Follow-ups from review of the git-refs ULID-emission PR (the id-kind read-routing item, #1, and the write-boundary kind guard, #2, are deferred — see below): - id.CheckpointID.DisplayShort() (new): kind-aware trim — legacy hex shows its 12-char prefix, a ULID is shown in full (front-truncating a ULID drops its entropy tail and yields an ambiguous, unresolvable prefix). - explain checkpoint-list view uses DisplayShort instead of a blind 12-char cut, so ULID checkpoints no longer collapse to near-identical timestamp prefixes. - blame --long sizes the Checkpoint/Session column to its content (attributionCheckpointColumnWidth) so a 26-char ULID + session renders whole instead of being clipped to a session-less 21-char prefix. - attach's fetch hint now resolves its target via resolveCheckpointFetchTarget — the same path FetchCheckpointRef uses — so the pasteable command matches the remote the fetch actually ran (fixes a bare "git fetch origin …" that fails in a token-only environment with an SSH origin). - id.MaxIDLength ties to oklog/ulid's EncodedSize instead of a third hardcoded 26. - eager-condense mints the checkpoint ID only after the skip checks, so a no-op session stop no longer pays the mint (and its checkpoints-config load). - docs/architecture/sessions-and-checkpoints.md: checkpoint IDs are 12-hex OR ULID (not fixed-width), minted via checkpoint.GenerateCheckpointID. Deferred to the id-kind read-routing follow-up (tracked separately): - #1 attach routes presence/refresh/fetch-hint by current config, not by the trailer ID's kind — after a git-branch⇄git-refs flip attach looks in the wrong place and can suggest an unfixable fetch. This is the read-routing work the PR body already names; the two concrete attach dead-ends belong in that issue. - #2 the ULID⇒refs invariant has no write-boundary guard. A naive "git-branch store saw a ULID → warn" misfires when git-branch is a *mirror* of a git-refs primary (a valid topology where ULIDs legitimately reach it), so the correct guard needs topology-role awareness and belongs with the routing work. Tests: TestCheckpointID_DisplayShort, TestAttributionCheckpointColumnWidth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 14006b64f8d1

Resolves the review packet on the Codex subagent change: - #1 Codex task-checkpoint UUID: documented that CheckpointUUID is Claude-format-specific (tool_result/UUID) and resolves to "" for Codex — rewind to a Codex task checkpoint restores files via the shadow tree but does not truncate the transcript (graceful fall-through; restore truncation is also Claude-only). Codex-aware line-offset truncation is a deferred follow-up. Comment at the call site + AGENT.md. - #2 Resumed-subagent token under-count: documented deliberate trade-off (chosen over the worse cross-turn double-count) + observability — logResumedOutOfRangeChildren debug-logs when a resume_agent targets a child spawned in a prior range. - #3 Regression tests: CalculateTotalTokenUsage nil-main-with-subagent-tokens unit test (guards the nil-deref); new integration test TestCodexSubagentEnd_SourcesFilesFromChildNotParent (+ SimulateCodexSubagentStart/Stop harness helpers) guarding handleLifecycleSubagentEnd's child-not-parent resolver. - #4 Debug logging across the discover→resolve→read→parse chain (readSubagentRolloutsUncached, CalculateTotalTokenUsage, spawn-output drop) so Codex wire-format drift is visible instead of silently zeroing attribution. - #5 isCodexSubagentRollout: nested source.subagent fallback for rollouts without thread_source + backward-compat test (missing marker → treated as parent turn). - #6 + nits: t.Parallel() on the pure-logic codex tests; AGENT.md PostToolUse line corrected (it IS consumed); StepTranscriptStart aligned to the resolved transcriptOffset (removes the exec/no-preState divergence; field is currently unconsumed by the strategy). go build (incl. integration tag), vet, unit + codex integration tests, lint (0) all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Soph added 3 commits January 5, 2026 14:48

cleanup more, make it owned by strategy

1489a88

Entire-Checkpoint: 94de498254e2

make sure we compare the same commit parts

dc7259f

Entire-Checkpoint: 9878b713483d

review feedback

158e36f

- make sure we compare the right parts of commit hashes - make sure we don't delete fresh created session states - log what gets cleaned up Entire-Checkpoint: 9878b713483d

Soph force-pushed the soph/move-clean-to-session-cleanup branch from 96de41c to 158e36f Compare January 5, 2026 16:46

Soph requested a review from dipree January 5, 2026 16:55

dipree approved these changes Jan 5, 2026

View reviewed changes

Soph merged commit a0c4a40 into main Jan 5, 2026
3 checks passed

Soph deleted the soph/move-clean-to-session-cleanup branch January 7, 2026 16:19

This was referenced May 8, 2026

auth: review follow-ups (provider routing, URL normalization, expiry preflight, HTTP timeouts) #1156

Merged

Pi: cold-attach native-store resolution, race fix, E2E coverage #1173

Merged

Soph mentioned this pull request May 31, 2026

entire: control-plane CLI via generated Core API client (org/project/repo/grant/mirror) #1299

Merged

6 tasks

computermode mentioned this pull request Jun 3, 2026

doctor: check and repair the v1.1 committed-read mirror #1350

Merged

Soph mentioned this pull request Jun 15, 2026

Pluggable checkpoint stores: abstraction refactor (tracking) #1433

Open

5 tasks

Soph mentioned this pull request Jun 30, 2026

Store full compacted transcript per checkpoint with a scope marker #1581

Merged

suhaanthayyil mentioned this pull request Jul 1, 2026

fix(experts): route to the repo's entire-api cell with an identity token #1588

Merged

Soph mentioned this pull request Jul 4, 2026

Route checkpoint reads by ID kind across git backends #1630

Merged

Mohit-Katyal mentioned this pull request Jul 4, 2026

feat(ticket): entire ticket — link external tickets to branches, checkpoints & reviews #1633

Open

Soph mentioned this pull request Jul 4, 2026

test: git-remote coverage (1/4) — plan + backend matrix, real-hook push, hermeticity #1636

Merged

karthik-rameshkumar mentioned this pull request Jul 16, 2026

entire enable/import while logged out imports locally with no way to sync after login #1773

Open

This was referenced Jul 20, 2026

fix(claudecode): extract model from transcript to fix Unknown attribution (#1804) #1805

Merged

Claude Code 2.1.x sessions attributed as 'Unknown' — transcript flush sentinel times out (3s) + empty model on checkpoints (v0.8.42) #1804

Closed

ColeDriver mentioned this pull request Jul 26, 2026

fix(strategy): warn when a commit is left unlinked with a live sibling-worktree session #1858

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move "clean" root command to "session cleanup", some refactoring#2

Move "clean" root command to "session cleanup", some refactoring#2
Soph merged 3 commits into
mainfrom
soph/move-clean-to-session-cleanup

Soph commented Jan 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Soph commented Jan 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants