Skip to content

Support Grok Build CLI as a provider - #23

Merged
wilbeibi merged 1 commit into
wilbeibi:mainfrom
rudironsoni:feature/grok-support
Sep 27, 2026
Merged

wilbeibi merged 1 commit into
wilbeibi:mainfrom
rudironsoni:feature/grok-support

Conversation

@rudironsoni

@rudironsoni rudironsoni commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Grok Build support

Read Grok Build sessions from $GROK_HOME/sessions through catchup's existing selection, filtering, rendering, and handoff commands. The authoritative updates.jsonl stream preserves user turns, timestamps, tool and background-task failures, error stops, and history across compaction. A checkpoint can contain multiple compaction_meta records; the reader includes all of them in order so it retains the actual summary as well as injected context.

chat_history.jsonl is the fallback when updates are absent, contain no readable conversation, or exceed 512 MiB. Fallback warnings name the missing timestamps, failures, stop reasons, and potentially missing pre-compaction history. Earlier parse warnings survive fallback. Real user turns with promptIndex remain visible; hideFromScrollback filters injected turns. Explicit hidden: false overrides the subagent default, while hidden sessions and unused husks remain accessible by ID.

Native fork uses grok --resume <id> --fork-session; cross-agent handoff uses grok [-m model] <prompt>. Both argument forms were checked against grok 1.0.41 (4220f3b224a6) --help. This verifies the supported flags, not a newly launched interactive fork.

Response to review

  • Based directly on current upstream main, 8cf6a47, including AGENTS.md.
  • Removed the shared conformance package, all provider-wide conformance tests, source/comment-scanning gates, and repository-wide PR template.
  • Restored unrelated Cursor documentation and Amp test/comment changes. Existing providers' implementation files match upstream. Shared runtime changes only register Grok and its roots/launch commands.
  • Reused session.Failure, query/summary helpers, root resolution, and common CLI/rendering. Removed the duplicate JSON compactor and unused decoded fields. No new dependencies.
  • Consolidated repeated parser assertions into provider-local tables and reused the sanitized chat fixture in the existing CLI test file. The original ignored docs/providers.md is not shipped or linked as documentation.

Changes against upstream, counting complete files by purpose:

Category Additions Deletions
Runtime code 845 3
Tests and fixtures 366 4
Documentation 4 4
Total 1,215 11

The PR changed 2,155 lines before revision and now changes 1,226, a reduction of 929 lines. These are PR-to-base counts; removed unrelated additions are not credited against the Grok implementation's cost.

Complexity and evidence

Lizard measurements compare upstream, the previous PR, and this revision. The Grok event dispatcher remains at cyclomatic complexity 40 and the chat reader at 18. Fallback selection rises from 6 to 11 because it now distinguishes filesystem errors, empty/damaged streams, and preservation of warnings. The event dispatcher and chat reader remain complex. Lower line counts and passing tests do not establish that this complexity is necessary. Existing provider selection and launch switches gain only their Grok cases.

The two transcript formats require separate decoding because ACP message chunks carry content objects, tool updates carry arrays or raw output, and chat history carries model-message records. Reading only the smaller chat file would lose the timestamped failure history and earlier turns this feature is meant to recover. The 512 MiB cap remains an explicit fallback policy, tested with a sparse file.

Fixtures are sanitized excerpts from a real session inspected with Grok 1.0.41. Their source line numbers and transformations are recorded in internal/grok/grok_test.go; ordering and relevant record shapes are retained. Visibility predicates were checked against xai-org/grok-build commit f0e3be1100ef5252488e3be8bb0e91cf68d8c305, crates/codegen/xai-grok-shell/src/session/persistence.rs. That source reference is separate from the installed binary revision.

Verification

Passed on macOS arm64 with Go 1.25.0:

  • go test -mod=readonly ./...
  • go test -mod=readonly -race ./...
  • go vet -mod=readonly ./...
  • go build -mod=readonly -o /private/tmp/catchup-pr23-after .
  • gofmt -l . and git diff --check

Also passed go build ./... on the installed Go 1.27.1 toolchain and a Windows amd64 cross-build. Windows tests were not executed locally.

A read of the same unchanged 118,770,944-byte real updates log produced 426 entries, including 19 user turns, 87 failures, 20 compaction entries and 6 stops. All entries have timestamps and there are no warnings. Independent source checks match the failure/user/stop counts and all checkpoint text. All 20 previously incomplete checkpoint summaries are fixed; every non-compaction entry is identical to the previous implementation. The final Go 1.25 read took about 2.1 seconds in one run; this is not a performance benchmark.

Test review:

  • Kept: TestRead owns ordered parsing, fallback losses, retained context, incomplete/unknown records, streamed text and duplicate failure handling. Its literal expectations come from the sanitized records, independently of production helpers. TestListAndResolve, TestVisibility, TestIndexFallbacks and TestReadSourceErrors own selection, source-backed visibility and read errors. Existing CLI/root test files cover the distinct integration and launch contracts.
  • Explored: 19 deliberate behavior mutations were caught, including first-only checkpoint extraction, hidden-flag precedence, lost warnings, hidden-turn leakage, chunk loss, background failure/stop loss, duplicate failures, timestamps, retained turns, query/recency/title selection, source validation, launch flags, handoff model placement and root overrides. All mutations were restored. The temporary mutation runner and live-log comparison are not added to the repository.
  • Proposed: no additional test infrastructure is included in this PR.

Remaining limits: compatibility with other Grok versions is unverified; oversized updates use the disclosed lossy fallback; missing/unreadable checkpoint files leave a bare compaction marker, with the existing --since-compact warning when applicable. Upstream CI run 36315598741 reports action_required with zero jobs. No upstream Linux/Windows test result is available; this is separate from the passing local checks and Windows cross-build.

@rudironsoni
rudironsoni force-pushed the feature/grok-support branch 2 times, most recently from d124dae to faafeb2 Compare September 26, 2026 13:03
@wilbeibi

Copy link
Copy Markdown
Owner

Thanks for contributing Grok Build support! Please rebase this PR onto the latest main and revise it against the new AGENTS.md contribution guide (added in 8cf6a47).

The main priorities are:

  • Keep the complexity-to-benefit ratio high and be conservative with added lines, including tests and fixtures. Since this PR exceeds 300 changed lines, please apply the linked complexity guide.
  • Keep the Grok addition focused. Please propose the shared conformance framework and repository-wide test gates separately.
  • Preserve existing providers' behavior, including when changing shared code.
  • Follow the linked test-writing guide: prefer extending existing fixtures and remove duplicate assertions. Ground parser expectations in real logs or upstream schemas/source.

Please include the resulting additions/deletions for runtime code, tests/fixtures, and docs, along with the verification performed and any remaining limits.

Read the authoritative ACP timeline under GROK_HOME through the existing
provider, CLI, and rendering interfaces. Preserve real user turns, failures,
stops, timestamps, and every compaction summary record. Warn about losses
when falling back to chat history and honor explicit session visibility.

Use sanitized real-record fixtures with focused provider and CLI tests.
Keep shared conformance infrastructure out of this provider addition.
@rudironsoni

rudironsoni commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor Author

@wilbeibi, I updated this PR in commit 2c38180 to follow your review and AGENTS.md.

The commit has main at 8cf6a47 as its parent. The PR description records the remaining complexity and verification limits.

I made these changes:

  • Removed the shared conformance framework, tests for other providers, repository-wide test gates, and PR template.
  • Restored the unrelated Amp and Cursor changes. The implementation files for existing providers match main.
  • Reused the existing failure, query, root, and CLI functions.
  • Combined duplicate tests. The fixtures contain records from Grok 1.0.41. I removed private information from these records. The test file records their source and changes.

I also corrected three Grok behaviors:

  • The reader includes all compaction summary records. Previously, it could omit the actual summary.
  • An explicit hidden: false value has priority over the default for a subagent session.
  • The reader preserves error warnings when it uses chat_history.jsonl. It warns about missing timestamps, tool failures, and stop reasons. It also warns about possible loss of history before compaction.

The final changes against main are:

Category Added lines Deleted lines
Runtime code 845 3
Tests and fixtures 366 4
Documentation 4 4

The total decreased from 2,155 to 1,226 changed lines. The PR description includes the remaining code complexity.

All tests, race checks, the build, and go vet passed on Go 1.25.0. Format and whitespace checks also passed. The Windows cross-build passed.

I made 19 temporary code changes that caused incorrect behavior. The tests detected all 19 changes. I then restored the correct code.

I compared both versions with the same real session. All 20 compaction summaries now include the missing content. All other entries are unchanged.

Upstream CI reports action_required. No CI jobs ran. I did not run the tests on Windows.

The PR description records the verification details and remaining limits.

@wilbeibi
wilbeibi merged commit ba42774 into wilbeibi:main Sep 27, 2026
@wilbeibi

Copy link
Copy Markdown
Owner

Merged — thanks for the thorough revision. I'll follow up in a separate commit on retained context under --since-compact and on trimming the fallback path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants