Repository navigation
refactor(shared): move the repository diff types into their own file - #1710
Merged
Merged
Conversation
A type-only slice (plan: .omo/plans/feature-slices-plan.md, section 7): RepoDiffScope, RepoDiffStatus, RepoDiffRepository, RepoDiffFile, RepoDiffFileList and RepoDiffFilePatch move verbatim from shared/protocol.ts into shared/repo-diff.ts (types only). Their routes were already split out (server/workspace-routes.ts, server/repo-diff.ts); protocol.ts re-exports the six names, so every importer compiles unchanged.
|
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 and concisely describes the main change: moving the repository diff types into a dedicated shared file. |
| Description check | |
The description explains the change, resulting import behavior, scope, runtime impact, validation results, and overlapping pull requests. It uses “What changes” and “Checks” instead of the template he… |
| 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: Description check
-
Explanation
The description explains the change, resulting import behavior, scope, runtime impact, validation results, and overlapping pull requests. It uses “What changes” and “Checks” instead of the template headings “Change” and “Validation,” but it provides the required information.
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 c36a3e8 with |
devswha
added a commit
that referenced
this pull request
Oct 23, 2026
#1711) A type-only slice of phase 2 (plan `.omo/plans/feature-slices-plan.md`, section 7, item 9), after CLIProxy (#1699), Portal (#1700), directories (#1701), terminal find (#1703), web push (#1707), usage (#1708) and repository diff (#1710); session history (item 7) waits while the open PR #1598 rewrites those types: the access and device types move out of `shared/protocol.ts` into `shared/access.ts`. Their routes were already split out (`server/access.ts`, `server/devices.ts` and `server/tailscale.ts`), so only the contract types were left in `protocol.ts`. ## What changes - **Moved:** `AccessVia`, `AccessRefusal`, `DeviceRole`, `HealthAuth`, `LanExposure`, `PairedDevice`, `PairingCode`, `RemoteAccess`, `TailscaleAccess`, verbatim with their doc comments, into the new `shared/access.ts`. The file holds types only, with no values. - **Stays in `protocol.ts`:** `export type { AccessVia, AccessRefusal, DeviceRole, HealthAuth, LanExposure, PairedDevice, PairingCode, RemoteAccess, TailscaleAccess } from "./access.ts";` at the old place, so every importer compiles unchanged, including `shared/machines.ts`, which imports `HealthAuth` from `protocol.ts` (the plan's check for this family). - **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 (138 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/access.ts`. - `HERDR_TEST_MODE=unit bun test server/access.test.ts server/devices.test.ts server/pairing.test.ts server/tailscale.test.ts`: **80 pass, 0 fail**. - `bun run check fast`: passes (unit 6161 pass, 0 fail, 12 skip in 440 files, 8 at a time, 57.6s). ## Overlapping open PRs Open PRs that touch `shared/protocol.ts`: #1706, #1652, #1598, #1442, #1427. I checked each diff for `AccessVia`, `AccessRefusal`, `DeviceRole`, `HealthAuth`, `LanExposure`, `PairedDevice`, `PairingCode`, `RemoteAccess`, `TailscaleAccess`: - #1706, #1652, #1598, #1442 do not change them. - #1427 edits `server/api.contract.test.ts`, which imports them from `protocol.ts`. That import still resolves through the re-export. Move-only: moved declarations plus one re-export line. No changelog fragment: users see no change.
devswha
added a commit
that referenced
this pull request
Oct 23, 2026
A type-only slice of phase 2 (plan `.omo/plans/feature-slices-plan.md`, section 7, item 10), after CLIProxy (#1699), Portal (#1700), directories (#1701), terminal find (#1703), web push (#1707), usage (#1708), repository diff (#1710) and access (#1711); session history (item 7) waits while the open PR #1598 rewrites those types: the pane layout types move out of `shared/protocol.ts` into `shared/pane-layout.ts`. Their routes were already split out (`server/pane-routes.ts`), so only the contract types were left in `protocol.ts`. ## What changes - **Moved:** `SplitPaneDirection`, `PaneDirection`, `SplitPaneRequest`, `PaneSplit`, `ZoomPaneRequest`, `PaneZoomed`, `FocusPaneRequest`, `SwapPaneRequest`, `PaneSwapped`, `ResizePaneRequest`, `PaneResized`, `SetSplitRatioRequest`, verbatim with their doc comments, into the new `shared/pane-layout.ts`. The file holds types only, with no values; it imports `PaneInfo` (for `PaneSplit`) from `./herdr-api.generated.ts`, as `protocol.ts` does. - **Stays in `protocol.ts`:** `export type { SplitPaneDirection, PaneDirection, SplitPaneRequest, PaneSplit, ZoomPaneRequest, PaneZoomed, FocusPaneRequest, SwapPaneRequest, PaneSwapped, ResizePaneRequest, PaneResized, SetSplitRatioRequest } from "./pane-layout.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 (159 lines). The other lines are the re-export in `protocol.ts` and the `PaneInfo` type import at the top of `shared/pane-layout.ts`. `protocol.ts` keeps its own `PaneInfo` import, which its other declarations still use. - **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/pane-layout.ts`. - These routes are covered by contract tests against an isolated live herdr (`server/pane-layout.contract.test.ts`, `server/api.contract.test.ts`, `server/route-envelope.contract.test.ts`), which CI's integration lane runs; the unit lane has none to run here. - `bun run check fast`: passes (unit 6161 pass, 0 fail, 12 skip in 440 files, 8 at a time, 57.4s). ## Overlapping open PRs Open PRs that touch `shared/protocol.ts`: #1706, #1652, #1598, #1442, #1427. I checked each diff for the twelve names: - #1706, #1652, #1442 do not change them. - #1598 edits `src/lib/machineContext.tsx`, which imports them from `protocol.ts`. That import still resolves through the re-export. - #1427 edits `site/demo/transport.ts`, which imports them from `protocol.ts`. That import still resolves through the re-export. Move-only apart from imports: moved declarations, one re-export line and one type import. No changelog fragment: users see no change.
devswha
added a commit
that referenced
this pull request
Oct 23, 2026
…s into their own file (#1713) 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: - CLIProxy (#1699) - Portal (#1700) - directories (#1701) - terminal find (#1703) - web push (#1707) - usage (#1708) - repository diff (#1710) - access (#1711) - pane layout (#1712) This slice moves the workspace, tab and worktree lifecycle types out of `shared/protocol.ts` into `shared/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:** `TabMoved` uses `HerdrTab`, which `protocol.ts` defines as an alias of the generated `TabInfo`. The new file imports `TabInfo as HerdrTab` from `./herdr-api.generated.ts`, so the moved lines stay unchanged. It does not import back from `protocol.ts`. - **Three places in `protocol.ts`:** the types sat in three places there. `WorkspaceMoved` sat by the snapshot types, and the others sat in two blocks on either side of `AgentRenameRequest`. - `AgentRenameRequest` stays, because it belongs to the agent rename route, not to this family. - `WorkspaceMoved`'s old place keeps `export type { WorkspaceMoved } from "./workspace-lifecycle.ts";`. - The first block's old place keeps one `export type { … } from "./workspace-lifecycle.ts";` for the other thirteen names. 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 252 lines as moved. The other lines are the two re-export lines in `protocol.ts` and the `TabInfo as HerdrTab` type import at the top of the new file. - **Client imports:** `bun run typecheck` passes on both tsconfigs. No file in `src/` or `site/` imports `shared/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: - #1706, #1652, #1442 do not change them. - #1598 edits `scripts/session-history-browser-qa.ts`, `src/components/SessionHistory.tsx`, `src/lib/machineContext.tsx` and `src/lib/sessionHistory.ts`, which import them from `protocol.ts`. Those imports still resolve through the re-export. - #1427 edits `server/api.contract.test.ts`, `server/workspace-routes.ts` and `site/demo/transport.ts`, which import them from `protocol.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.
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.
A type-only slice of phase 2 (plan
.omo/plans/feature-slices-plan.md, section 7, item 8), after CLIProxy (#1699), Portal (#1700), directories (#1701), terminal find (#1703), web push (#1707) and usage (#1708); session history (item 7) waits while the open PR #1598 rewrites those types: the repository diff types move out ofshared/protocol.tsintoshared/repo-diff.ts. Their routes were already split out (server/workspace-routes.tsandserver/repo-diff.ts), so only the contract types were left inprotocol.ts.What changes
RepoDiffScope,RepoDiffStatus,RepoDiffRepository,RepoDiffFile,RepoDiffFileList,RepoDiffFilePatch, verbatim with their doc comments, into the newshared/repo-diff.ts. The file holds types only, with no values.protocol.ts:export type { RepoDiffScope, RepoDiffStatus, RepoDiffRepository, RepoDiffFile, RepoDiffFileList, RepoDiffFilePatch } from "./repo-diff.ts";at the old place, so every importer compiles unchanged.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 the declarations and their comments as moved (140 lines). The only other line is the re-export inprotocol.ts.bun run typecheckpasses on both tsconfigs, so every importer still compiles through the re-export. No file insrc/orsite/importsshared/repo-diff.ts.HERDR_TEST_MODE=unit bun test server/repo-diff.test.ts src/lib/repoDiff.test.ts src/lib/diff.test.ts: 36 pass, 0 fail.bun run check fast: passes (unit 6161 pass, 0 fail, 12 skip in 440 files, 8 at a time, 57.3s).Overlapping open PRs
Open PRs that touch
shared/protocol.ts: #1706, #1652, #1598, #1442, #1427. I checked each diff forRepoDiffScope,RepoDiffStatus,RepoDiffRepository,RepoDiffFile,RepoDiffFileList,RepoDiffFilePatch:server/api.contract.test.tsandsite/demo/transport.ts, which import them fromprotocol.ts. Those imports still resolve through the re-export.Move-only: moved declarations plus one re-export line. No changelog fragment: users see no change.