Skip to content

refactor(server): move the shared conversation types into server/conversation-shared.ts - #1763

Merged
devswha merged 1 commit into
mainfrom
refactor/conversation-shared-module
Oct 23, 2026
Merged

refactor(server): move the shared conversation types into server/conversation-shared.ts#1763
devswha merged 1 commit into
mainfrom
refactor/conversation-shared-module

Conversation

@devswha

@devswha devswha commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Wave C2 of the per-agent module split. The plan is .omo/plans/agent-modules-split.md in the maintainer's workspace, and plan-reviewer approved it. It follows C1 (#1760) and P1b (#1761).

This wave moves the conversation types and constants that agent modules share out of server/conversation.ts into a new leaf module, server/conversation-shared.ts. Later waves (Devin's session lookup, then the Claude and omp sessions) can then throw ConversationUnavailable without importing conversation.ts.

What changes

  • Moved verbatim, with their doc comments:
    • TRANSCRIPT_WINDOW_BYTES;
    • the error classes ConversationUnavailable, ConversationNotStarted and HistoryChanged;
    • the types RecognizedConversation, StreamSource and ConversationPage;
    • PROCESS_VERSION and answerVersion.
  • State: PROCESS_VERSION holds one randomUUID(), set once and never changed. It moves with its only user, answerVersion, so it still has a single owner.
  • New export: only StreamSource gains one, because conversation.ts imports it back.
  • Re-exports: conversation.ts re-exports every name that was exported before, so no other importer changes.
  • One import path changes on purpose: server/hermes.ts now imports the same names from ./conversation-shared.ts instead of ./conversation.ts. That removes the runtime import cycle between hermes.ts and conversation.ts.

Checks

  • --color-moved: git diff --color-moved=plain --color-moved-ws=allow-indentation-change origin/main...HEAD shows 138 lines as moved. The other 8 lines are:
    • the new file's two import lines;
    • StreamSource with its added export, shown as removed and added;
    • conversation.ts's import and re-export lines;
    • hermes.ts's changed module path.
  • No cycles: checked with bun <plan>/no-path.ts.
    • With server/conversation-shared.ts server/conversation.ts server/prompt.ts, it prints "no path to server/conversation.ts, server/prompt.ts".
    • With server/hermes.ts server/conversation.ts, it prints "no path to server/conversation.ts". The cycle is gone.
  • Typecheck: bun run typecheck passes.
  • Unit tests: HERDR_TEST_MODE=unit bun test server/hermes.test.ts server/conversation.test.ts server/chat-history.test.ts: 177 pass, 0 fail.
  • bun run check fast: passes (unit 6253 pass, 0 fail, 13 skip in 443 files, 8 at a time, 66.5s)
  • Integration lane: CI runs the gjc, api, opencode and hermes contracts.

Outside PRs that touch the same files

No other maintainer PR touches these files. Three outside contributors' PRs touch server/conversation.ts. Each adds a member to RecognizedConversation's source union, which now lives in server/conversation-shared.ts, so a rebase carries that one line into the new file. I checked each with git merge-tree:

Move-only: moved declarations plus imports, one re-export line, one added export, and the hermes.ts module path. No changelog fragment: users see no change.

@devswha
refactor(server): move the shared conversation types into server/conv…
c28904b 
…ersation-shared.ts

Move the transcript window constant, the conversation error classes, RecognizedConversation,
ConversationPage, StreamSource, PROCESS_VERSION and answerVersion verbatim into the new leaf.
StreamSource gains an export; conversation.ts imports what it uses and re-exports the
previously exported names, so every importer stays unchanged. hermes.ts imports the leaf,
which removes the hermes.ts/conversation.ts import cycle.
Plan: .omo/plans/agent-modules-split.md C2.
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between d736944 and c28904b.

📒 Files selected for processing: 3
  • server/conversation-shared.ts
  • server/conversation.ts
  • server/hermes.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Refactor
    • Consolidated conversation paging, history-change, and availability handling into shared definitions. Existing conversation APIs continue to expose the same declarations, and no runtime behavior changes are included.
    • Updated related server components to use the shared conversation definitions. This update does not change user-visible behavior.

The change moves conversation paging definitions, errors, types, and version hashing into conversation-shared.ts. conversation.ts imports and re-exports them, and hermes.ts imports conversation symbols from the shared module.

Changes: Conversation shared definitions

Layer / File(s) Summary
Define and extract conversation primitives
server/conversation-shared.ts, server/conversation.ts		 conversation-shared.ts defines conversation errors, types, paging definitions, and answerVersion. conversation.ts removes the corresponding local declarations.
Wire shared definitions to consumers
server/conversation.ts, server/hermes.ts		 conversation.ts imports and re-exports the shared definitions. hermes.ts changes its import source to conversation-shared.ts; imported names remain unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to c2890

The extraction preserves the existing conversation behavior and public exports. No merge-blocking issue was identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: moving shared conversation types into server/conversation-shared.ts.
Description check Passed The description explains the change, resulting module structure, import-cycle removal, compatibility behavior, and validation results. It uses "What changes" and "Checks" instead of the template's "Ch…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the change, resulting module structure, import-cycle removal, compatibility behavior, and validation results. It uses "What changes" and "Checks" instead of the template's "Change" and "Validation" headings, but all required information is present.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devswha

devswha commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

Merge pass: move-only PR (per-agent split, wave C2), merged on its checks without waiting for a CodeRabbit review, as AGENTS.md allows.

  • Line comparison. Comparing removed and added lines, the only differences are:
    • imports and re-exports;
    • export added to StreamSource;
    • hermes.ts's import path.
  • Gate review. A gate reviewer on a different model than the author's confirmed:
    • The error classes keep one identity through both import paths, and no module mock or class spy diverges.
    • PROCESS_VERSION has a single owner, and both crypto imports are still used.
    • The hermes ↔ conversation cycle is gone, and no top-level code depended on the old load order.
    • A runtime probe passed for class identity, inheritance, answerVersion consistency and Hermes error recognition.
  • Tests at this head. 177 hermes, conversation and chat-history tests pass, the typecheck passes, and 47 conventions tests pass.
  • Outside contributors. feat(chat): read a droid pane's own session as its conversation #1442, feat(chat): read native Copilot CLI transcripts #1652 and feat(chat): read native Muse Code conversations #1706 touch server/conversation.ts. At most they need a rebase.

All required checks are green and no review thread is open.

@devswha
devswha merged commit d6bf59b into main Oct 23, 2026
17 checks passed
@devswha
devswha deleted the refactor/conversation-shared-module branch October 23, 2026 13:27
@devswha devswha mentioned this pull request Oct 23, 2026
Merged
devswha added a commit that referenced this pull request Oct 23, 2026
@devswha
refactor(server): move turn boundaries and the Devin session lookup i…
1b838cb 
…nto their own modules (#1765)

Wave C3 of the per-agent module split. The plan is
`.omo/plans/agent-modules-split.md` in the maintainer's workspace, and
plan-reviewer approved it. It follows C2 (#1763), which this wave builds
on: `StreamSource` and `ConversationUnavailable` now come from
`server/conversation-shared.ts`.

This wave moves two state-free clusters out of `server/conversation.ts`
into new leaf modules. `conversation.ts` keeps every cache, `paneReads`
(including Devin's per-pane reads), the paging core and the transcript
resolvers. The three functions tests spy on (`paneTranscript`,
`readCommandTranscript`, `paneProcessRoots`) stay.

## What changes

- **`server/transcript-turns.ts`:** the turn-boundary cluster, verbatim
with its doc comments:
- `ClaudeBranch`, `claudeForks`, `answeredBetween`,
`claudeCommandEchoRecord`, `CLAUDE_CHAIN_LINE_BYTES`,
`followClaudeBranch`;
  - `PEER_MARK`, `TURN_MARK`, `opensTurn`;
  - `PromptChain`, `turnStarts`.

`followClaudeBranch` calls `opensTurn`, which `turnStarts` also needs,
so the cluster moves as one module.

`export` is added to the five names `conversation.ts` still uses:
`turnStarts`, `followClaudeBranch`, `claudeForks`, and the types
`ClaudeBranch` and `PromptChain`.
- **`server/devin-session.ts`:** `isDevinProcess` and
`devinSessionForPane`, verbatim. Both were already exported.
`conversation.ts` re-exports them for `server/session-history.ts` and
`server/conversation.test.ts`, so no importer changes.
- **`conversation.ts` imports:** the imports from
`./transcript-records.ts` and `./claude-transcript.ts` drop the names
only the moved code used.
- **Not in this wave:** `rememberPaneDevin` and `DevinPaneRead` stay,
because they are fields of `paneReads`, which
`forgetPaneTranscriptState` owns.

`server/conversation.ts` goes from 1,732 to 1,532 lines.

## Checks

- **`--color-moved`:** `git diff --color-moved=plain
--color-moved-ws=allow-indentation-change origin/main...HEAD` shows 394
lines as moved. The other 23 lines are:
  - the two new files' imports;
- `conversation.ts`'s narrowed and new imports and its one re-export
line;
- the five declarations whose only change is the added `export`, each
shown as removed and added.
- **No import cycles:** `bun <plan>/no-path.ts <new file>
server/conversation.ts server/prompt.ts` prints "no path to
server/conversation.ts, server/prompt.ts" for both new files. They
import only `conversation-shared.ts`, `transcript-records.ts`,
`claude-transcript.ts` and `shared/protocol.ts`.
- **No new herdr calls:** neither new file calls `herdrRpc`.
- **Typecheck:** `bun run typecheck` passes.
- **Unit tests:** **305 pass, 0 fail across 10 files**, from:

`HERDR_TEST_MODE=unit bun test server/claude-turn-boundaries.test.ts
server/native-incremental.test.ts server/chat-history.test.ts
server/pi.test.ts server/pi-tree.test.ts server/pi-large-history.test.ts
server/grok.test.ts server/cmd.test.ts server/conversation.test.ts
server/devin.test.ts`
- `bun run check fast`: passes (unit 6253 pass, 0 fail, 13 skip in 443
files, 8 at a time, 58.7s)
- **Integration lane:** runs in CI.

## Outside PRs that touch the same files

No other maintainer PR touches these files. Three outside contributors'
PRs touch `server/conversation.ts`: #1706, #1652 and #1442.
- **Status:** each already conflicts with main there, so it needs a
rebase either way.
- **What the rebase carries:** #1706, #1652 and #1442 edit `TURN_MARK`,
`opensTurn` or `followClaudeBranch`. Those edits move to
`server/transcript-turns.ts` in the rebase.

Move-only: moved declarations plus imports, five added `export`s and one
re-export line. No changelog fragment: users see no change.
@devswha devswha mentioned this pull request Oct 24, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@devswha