Repository navigation
refactor(shared): move the workspace, tab and worktree lifecycle types into their own file - #1713
Merged
Merged
Conversation
…s into their own file A type-only slice (plan: .omo/plans/feature-slices-plan.md, section 7): the create, move, open and remove request and answer types of workspaces, tabs and worktrees, and WorkspaceMoved, move verbatim from shared/protocol.ts into shared/workspace-lifecycle.ts, which imports TabInfo as HerdrTab from the generated herdr types (the alias protocol.ts defines), so TabMoved reads as before. They sat in three places: WorkspaceMoved by the snapshot types, and two blocks around AgentRenameRequest, which belongs to the agent rename route and stays. protocol.ts re-exports the names at their old places, so every importer compiles unchanged. Their routes were already split out (server/workspace-routes.ts).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly summarizes the main change: moving workspace, tab, and worktree lifecycle types into a separate file. |
| Description check | |
The description explains the change and resulting behavior. It also lists validation checks and results, including type checking and tests. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.
✨ Finishing Touches
-
📝 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.
Comment @coderabbitai help to get the list of available commands.
Owner
Author
|
Merging as a move-only PR (AGENTS.md), without waiting for CodeRabbit. The merge pass checked head 205d0db with |
devswha
added a commit
that referenced
this pull request
Oct 23, 2026
…1753) The last type-only slice of phase 2 (plan `.omo/plans/feature-slices-plan.md`, section 7, item 7). It follows the other type-only slices (#1699 to #1713) and waited for the session resume PR (#1598), which added `SessionResumeSource`, `SessionResumeRequest` and `SessionResumeResponse` beside the history types. The session history types move out of `shared/protocol.ts` into `shared/session-history.ts`. Their routes were already split out (`server/session-history-routes.ts`), so only the contract types were left in `protocol.ts`. ## What changes - **Moved:** `SessionHistoryAgent`, `SessionResumeSource`, `SessionHistoryRow`, `SessionResumeRequest`, `SessionResumeResponse`, `SessionHistoryResponse`, verbatim with their doc comments, into the new `shared/session-history.ts`. The file holds types only, with no imports and no values. - **Stays in `protocol.ts`:** `export type { SessionHistoryAgent, SessionResumeSource, SessionHistoryRow, SessionResumeRequest, SessionResumeResponse, SessionHistoryResponse } from "./session-history.ts";` at the old place, so every importer compiles unchanged. - **No other `protocol.ts` declaration uses these names**, so nothing else had to stay behind. There is no behaviour to pin: types are erased at runtime, and the routes and their tests are untouched. ## Checks - **`--color-moved`:** `git diff --color-moved=plain origin/main...HEAD -- shared` shows the declarations and their comments as moved (96 lines). The only other line is the re-export in `protocol.ts`. - **Client imports:** `bun run typecheck` passes on both tsconfigs, so every importer still compiles through the re-export. No file in `src/` or `site/` imports `shared/session-history.ts`. - `HERDR_TEST_MODE=unit bun test server/session-history.test.ts server/machine-proxy.test.ts src/lib/sessionHistory.test.ts server/claude-store.test.ts`: **190 pass, 0 fail**. The routes' contract tests (`server/api.contract.test.ts`, `server/device-access.contract.test.ts`) run in CI's integration lane. - `bun run check fast`: passes (unit 6244 pass, 0 fail, 13 skip in 442 files, 8 at a time, 57.6s) ## Outside PRs that touch the same files No other maintainer PR touches these files. Outside contributors' PRs that touch `shared/protocol.ts`: #1714, #1706, #1652, #1442 and #1427. - None of them changes the moved types, so none needs code changes, at most a rebase. - I tested each against this branch with `git merge-tree`: - #1714, #1706 and #1652 still merge cleanly in `shared/`. - #1442 and #1427 already conflict with main in `shared/protocol.ts`. This move adds no new conflict there. - Their imports from `protocol.ts` keep resolving through the re-export. Move-only: moved declarations plus one re-export line. No changelog fragment: users see no change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The last type-only slice of phase 2 that can go now (plan
.omo/plans/feature-slices-plan.md, section 7, item 11). It follows:This slice moves the workspace, tab and worktree lifecycle types out of
shared/protocol.tsintoshared/workspace-lifecycle.ts. Their routes were already split out (server/workspace-routes.ts). Session history (item 7) waits while the open PR #1598 rewrites those types.What changes
Moved, verbatim with their doc comments, into the new
shared/workspace-lifecycle.ts:WorkspaceMoved;CreateWorkspaceRequest,CreateTabRequest,CreateWorktreeRequest;MoveTabRequest,TabMoved;OpenWorktreeRequest,WorktreeOpened;RemoveWorktreeRequest,WorktreeRemoved;WorktreeEntry,WorktreeListing;WorkspaceCreated,TabCreated.The file holds types only, with no values.
One import in the new file:
TabMovedusesHerdrTab, whichprotocol.tsdefines as an alias of the generatedTabInfo. The new file importsTabInfo as HerdrTabfrom./herdr-api.generated.ts, so the moved lines stay unchanged. It does not import back fromprotocol.ts.Three places in
protocol.ts: the types sat in three places there.WorkspaceMovedsat by the snapshot types, and the others sat in two blocks on either side ofAgentRenameRequest.AgentRenameRequeststays, because it belongs to the agent rename route, not to this family.WorkspaceMoved's old place keepsexport type { WorkspaceMoved } from "./workspace-lifecycle.ts";.export type { … } from "./workspace-lifecycle.ts";for the other thirteen names.Every importer compiles unchanged.
No other
protocol.tsdeclaration uses these names, so nothing else had to stay behind.There is no behaviour to pin: types are erased at runtime, and the routes and their tests are untouched.
Checks
--color-moved:git diff --color-moved=plain origin/main...HEAD -- sharedshows 252 lines as moved. The other lines are the two re-export lines inprotocol.tsand theTabInfo as HerdrTabtype import at the top of the new file.bun run typecheckpasses on both tsconfigs. No file insrc/orsite/importsshared/workspace-lifecycle.ts.HERDR_TEST_MODE=unit bun test server/order-sequence.test.ts scripts/demo-tabs.test.ts scripts/demo-first-message.test.ts src/lib/workspaceOrder.test.ts: 42 pass, 0 fail. The routes themselves are covered by contract tests against an isolated live herdr (server/api.contract.test.ts,server/route-envelope.contract.test.ts,server/pending-input.contract.test.ts), which CI's integration lane runs.bun run check fast: passes (unit 6161 pass, 0 fail, 12 skip in 440 files, 8 at a time, 57.5s).Overlapping open PRs
Open PRs that touch
shared/protocol.ts: #1706, #1652, #1598, #1442, #1427. I checked each diff for the fourteen names:scripts/session-history-browser-qa.ts,src/components/SessionHistory.tsx,src/lib/machineContext.tsxandsrc/lib/sessionHistory.ts, which import them fromprotocol.ts. Those imports still resolve through the re-export.server/api.contract.test.ts,server/workspace-routes.tsandsite/demo/transport.ts, which import them fromprotocol.ts. Those imports still resolve through the re-export.Move-only apart from imports: moved declarations, two re-export lines and one type import. No changelog fragment: users see no change.