Repository navigation
refactor(shared): move the directory browser types into their own file - #1701
Merged
Merged
Conversation
A type-only slice (plan: .omo/plans/feature-slices-plan.md, section 7): DirectoryListing, CreateDirectoryRequest and DirectoryCreated move verbatim from shared/protocol.ts into shared/directories.ts (types only). Their route was already split out (server/workspace-routes.ts, server/directories.ts); protocol.ts re-exports the three 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 "refactor(shared): move the directory browser types into their own file" matches the change. The PR moves DirectoryListing, CreateDirectoryRequest, and DirectoryCreated from shared/protocol.… |
| Description check | |
The description covers the required sections of the template. The Change section explains the problem and the resulting behavior: the types move to a new file, protocol.ts re-exports them, and no runt… |
| 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: Title check
-
Explanation
The title "refactor(shared): move the directory browser types into their own file" matches the change. The PR moves DirectoryListing, CreateDirectoryRequest, and DirectoryCreated from shared/protocol.ts into shared/directories.ts and re-exports them from protocol.ts. The title is specific and concise.
Full details: Description check
-
Explanation
The description covers the required sections of the template. The Change section explains the problem and the resulting behavior: the types move to a new file, protocol.ts re-exports them, and no runtime behavior changes. The Validation section lists typecheck, unit tests, the color-moved diff check, and bun run check fast results. The template asks for contract-test coverage for HTTP/WS changes. This PR is type-only and leaves the routes untouched, so that item does not apply. The description references CONTRIBUTING.md's changelog rule and explains why no changelog fragment was added. The description is complete.
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). The merge pass checked head bcd4f39 with |
devswha
added a commit
that referenced
this pull request
Oct 23, 2026
…1703) A type-only slice of phase 2 (plan `.omo/plans/feature-slices-plan.md`, section 7, item 4), after CLIProxy (#1699), Portal (#1700) and directories (#1701): the terminal find types move out of `shared/protocol.ts` into `shared/find.ts`. Their routes were already split out (`server/pane-routes.ts`), so only the contract types were left in `protocol.ts`. ## What changes - **Moved:** `PaneFindMatch`, `PaneFindRequest`, `PaneFindResponse`, verbatim with their doc comments, into the new `shared/find.ts`. The file holds types only, with no values; it imports `PaneScrollInfo` from `./herdr-api.generated.ts`, as `protocol.ts` did. - **Stays in `protocol.ts`:** `export type { PaneFindMatch, PaneFindRequest, PaneFindResponse } from "./find.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 (56 lines). The other lines are the re-export in `protocol.ts`, the `PaneScrollInfo` type import at the top of `shared/find.ts`, and `protocol.ts`'s own local type import from the generated file without `PaneScrollInfo`, which only these types used there. `PaneScrollInfo` stays re-exported from `protocol.ts` as before. - **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/find.ts`. - `HERDR_TEST_MODE=unit bun test src/lib/paneFind.test.ts scripts/demo-history.test.ts`: **6 pass, 0 fail**. - `bun run check fast`: passes (unit 6161 pass, 0 fail, 12 skip in 440 files, 8 at a time, 57.7s). ## Overlapping open PRs Open PRs that touch `shared/protocol.ts`: #1652, #1598, #1442, #1427. I checked each diff for `PaneFindMatch`, `PaneFindRequest` and `PaneFindResponse`: - #1652, #1598, #1442 do not change them. - #1427 edits `server/api.contract.test.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, one re-export line, and the two type-import lines above. 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 5), after CLIProxy (#1699), Portal (#1700), directories (#1701) and terminal find (#1703): the web push types move out of `shared/protocol.ts` into `shared/push.ts`. Their routes were already split out (`server/push.ts`), so only the contract types were left in `protocol.ts`. ## What changes - **Moved:** `PushKey`, `PushPayload`, verbatim with their doc comments, into the new `shared/push.ts`. The file holds types only, with no values. - **Stays in `protocol.ts`:** `export type { PushKey, PushPayload } from "./push.ts";` at the old place, so every importer compiles unchanged. The two types sat in two places in `protocol.ts` (`PushKey` near the top, `PushPayload` beside the WebSocket messages); both now come from this one line at `PushKey`'s old place. - **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 (43 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/push.ts`. - `HERDR_TEST_MODE=unit bun test server/push.test.ts src/lib/notifications.test.ts`: **50 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 for `PushKey` and `PushPayload`: - #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 6), after CLIProxy (#1699), Portal (#1700), directories (#1701), terminal find (#1703) and web push (#1707): the usage types move out of `shared/protocol.ts` into `shared/usage.ts`. Their routes were already split out (`server/usage.ts`), so only the contract types were left in `protocol.ts`. ## What changes - **Moved:** `UsageProviderId`, `UsageWindow`, `UsageProblem`, `ProviderUsage`, `UsageReport`, verbatim with their doc comments, into the new `shared/usage.ts`. The file holds types only, with no values. - **Stays in `protocol.ts`:** `export type { UsageProviderId, UsageWindow, UsageProblem, ProviderUsage, UsageReport } from "./usage.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/usage.ts`. - `HERDR_TEST_MODE=unit bun test server/usage.test.ts src/lib/usage.test.ts`: **68 pass, 0 fail**. - `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 `UsageProviderId`, `UsageWindow`, `UsageProblem`, `ProviderUsage`, `UsageReport`: - #1706, #1652, #1598, #1442 do not change them. - #1427 edits `server/api.contract.test.ts` and `site/demo/transport.ts`, which import them from `protocol.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.
devswha
added a commit
that referenced
this pull request
Oct 23, 2026
…1710) 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 of `shared/protocol.ts` into `shared/repo-diff.ts`. Their routes were already split out (`server/workspace-routes.ts` and `server/repo-diff.ts`), so only the contract types were left in `protocol.ts`. ## What changes - **Moved:** `RepoDiffScope`, `RepoDiffStatus`, `RepoDiffRepository`, `RepoDiffFile`, `RepoDiffFileList`, `RepoDiffFilePatch`, verbatim with their doc comments, into the new `shared/repo-diff.ts`. The file holds types only, with no values. - **Stays in `protocol.ts`:** `export type { RepoDiffScope, RepoDiffStatus, RepoDiffRepository, RepoDiffFile, RepoDiffFileList, RepoDiffFilePatch } from "./repo-diff.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 (140 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/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 for `RepoDiffScope`, `RepoDiffStatus`, `RepoDiffRepository`, `RepoDiffFile`, `RepoDiffFileList`, `RepoDiffFilePatch`: - #1706, #1652, #1442, #1427 do not change them. - #1598 edits `server/api.contract.test.ts` and `site/demo/transport.ts`, which import them from `protocol.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.
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 3), after CLIProxy (#1699) and Portal (#1700): the directory browser types move out ofshared/protocol.tsintoshared/directories.ts. Their routes were already split out (server/workspace-routes.tsandserver/directories.ts), so only the contract types were left inprotocol.ts.What changes
DirectoryListing,CreateDirectoryRequest,DirectoryCreated, verbatim with their doc comments, into the newshared/directories.ts. The file holds types only, with no values.protocol.ts:export type { DirectoryListing, CreateDirectoryRequest, DirectoryCreated } from "./directories.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 (64 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/directories.ts.HERDR_TEST_MODE=unit bun test server/directories.test.ts: 6 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: #1652, #1598, #1442, #1427. I checked each diff forDirectoryListing,CreateDirectoryRequestandDirectoryCreated:server/workspace-routes.ts, which imports them fromprotocol.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.