Repository navigation
chore: Restructure the Go CLI into independent dispatcher, runner, and shared modules - #1461
Merged
Merged
Conversation
The release commit search matched any commit whose pathspec-limited diff re-added the manifest entry and the changelog heading. A commit that moves a package changelog always re-adds every changelog line in that diff because the rename source falls outside the pathspec, so a restructure commit could impersonate the release commit and break the sync against the published release target. Reuse is-release-please-release-commit.sh so only release-please subjects qualify, ahead of the directory split that moves cli/CHANGELOG.md.
The directory split will turn cli into multiple Go modules (common, project-runner, dispatcher, release tooling). A workspace lets every module resolve its siblings locally without publishing, and landing it first pins that the existing single-module checks stay green with a go.work present. The go directive matches cli/go.mod so the patch-level toolchain pin stays solely in cli/.go-version.
Move clicore, project, skills, tools, unityipc, and version out of cli/internal into a new common module that depends on no other module in the repo, so the compiler itself enforces the shared-code boundary once dispatcher and project-runner become separate modules. The runner contract (contract.json and its loader) moves to common/clicontract because common code (clicore, unityipc) and the dispatcher both consume the runner version and protocol generation, and neither may depend on the runner module; the dispatcher contract stays at the cli module root until the dispatcher module split. Release-please extra-files now point at the moved stamped files via repo-root-relative paths.
Move internal/dispatcher, install, uninstall, update, cmd/dispatcher, and the dispatcher contract out of the cli module into a dispatcher module that requires only common. The compiler now enforces that the dispatcher and the project runner cannot import each other: neither module requires the other. The dispatcher contract package is renamed to dispatchercontract because the clicontract name now belongs to the runner contract in common. Exclude-paths for the moved directories are dropped from the runner release config since the paths left its package root, and the boundary architecture tests now run go list inside the owning module.
Move internal/automation and the six release automation commands out of the cli module into tools/release-automation, requiring only common. Path constants inside the guards now describe the split layout (common/clicontract, dispatcher/), and the runner and dispatcher contracts read at git refs fall back to the legacy cli/ paths because releases tagged before the split still store the contracts there. Workflow steps and the release sync script now run the automation commands from the new module directory, and the cli package exclude list is gone because the excluded directories left the package root.
Every dispatcher source file moved into the dispatcher module in this branch, which changes the dispatcher release inputs, so the next dispatcher release must carry a new version per the bump guard.
The dispatcher version bump guard read the base dispatcher contract only at the split path, so any base ref predating the directory split looked like an initial contract introduction and the bump requirement was silently skipped. Route the base read through the existing legacy-path fallback so pre-split bases still enforce a version increase; a base missing the contract at both paths remains the bootstrap case.
…path The dispatcher module split moved dispatcher-contract.json from cli/ to dispatcher/, and resolve-dispatcher-release-target.sh now reads the new path. The test fixture still wrote the contract to the legacy cli/ path, so every case failed with a missing-file jq error.
…e repo root After the dispatcher and release-automation splits, the cli/ directory only contained the project runner, so the name no longer described the module. Renaming the directory and Go module path finishes the physical split of the release boundaries. - release-please package key "cli" -> "project-runner"; the component uloop-project-runner and its tag series stay unchanged, and the manifest carries the current version over to the renamed key - native binaries now build into a repo-root dist/ tree shared by the dispatcher and project runner outputs - the Go toolchain version file, golangci configs, and the layout contract (now schema v2 declaring all four modules) move to the repo root as the single source of truth - workflows, packaging, release-target, and sync scripts follow the new paths; module-wide checks and vulnerability scans cover all four modules
Contributor
|
Important Review skippedToo many files! This PR contains 206 files, which is 56 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (11)
📒 Files selected for processing (206)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
hatayama
marked this pull request as ready for review
July 2, 2026 13:23
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 209 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/resolve-native-cli-release-target.sh">
<violation number="1" location="scripts/resolve-native-cli-release-target.sh:15">
P2: Project-runner assets can be skipped even when workspace dependency wiring changes, because release-input detection no longer includes repo-root go.work. Adding go.work (and go.work.sum if present) to CLI_RELEASE_INPUT_PATHS would keep publish decisions aligned with actual build inputs.</violation>
</file>
<file name="tools/release-automation/internal/automation/dispatcher_version_bump_guard.go">
<violation number="1" location="tools/release-automation/internal/automation/dispatcher_version_bump_guard.go:20">
P2: Dispatcher behavior changes in `common/project` or `common/version` can bypass this bump guard, so dispatcher releases may merge without a required `dispatcherVersion` increment. Expanding the common-module patterns beyond `common/clicore` would keep the guard aligned with current dispatcher dependencies.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Re-trigger cubic
The directory split moved built binaries from cli/dist to the repo-root dist tree, but defaultUloopPath still joined the legacy cli segment, so running the smoke without --uloop-path or ULOOP_BIN always failed with a missing-binary error. The string never matched the cli/dist residual grep because filepath.Join splits it into separate segments.
The complexity job triggers on changes in every module, but after the split both the script and the workflow only linted project-runner, so complexity in common, dispatcher, and tools/release-automation went silently unmeasured. Before the split the single cli module covered all of that code. The workflow writes one JSON artifact per module, and the script keeps a fatal golangci-lint status from being masked by a later module that only reports findings.
runnerContractFileAtRef and dispatcherContractFileAtRef implemented the same try-primary-then-legacy-path branching with only the file constants differing. A single contractFileAtRefWithLegacyFallback keeps the two readers from drifting when the fallback behavior changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…-split-go-work # Conflicts: # .release-please-manifest.json
The architecture test now applies the 500-line production file cap to every module, which surfaced protocol_minimum_version_guard.go at 523 lines. The git contract reading layer (legacy-path fallback and command execution) is a distinct concern also consumed by the dispatcher guard, so move it to its own file instead of raising the cap.
After the module split, the architecture test could only see the project-runner module from its old location, leaving stale never-match boundary lists. Relocate it to the release-automation module and anchor everything at the repository root so it enforces the whole-repo layout: - the pre-split top-level cli/ directory must not reappear - module dependency directions: common requires no repo module, the other three require only common (acceptance criterion 7) - go directive alignment across go.mod files, go.work, and .go-version - production file size cap and internal boundary lists now cover all four modules; common must never grow internal packages - layout-contract v2 checks now include the Windows binary names, and module enumeration is cross-checked against go.work, check scripts, and the code-complexity workflow Each new guard was proven to fail via fake violation injection.
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The module split moved every path these documents referenced: the IPC contract now lives at common/clicontract/contract.json, the dispatcher contract under dispatcher/, the bump guard under tools/release-automation, and development binaries under the repo-root dist/. The Unity-side constant is now MINIMUM_REQUIRED_PROJECT_RUNNER_VERSION. Refresh AGENTS.md (CLAUDE.md is a symlink to it), README, the CodeRabbit IPC instructions, and the simplification plan's verification commands so agents and reviewers stop being pointed at pre-split paths.
Current git (measured on 2.50.1) reports a file absent from both the ref and the working tree as "fatal: path '...' does not exist in ...", but the classifier only matched the capitalized "Path" form from older git versions. In environments without the file on disk (bare or sparse CI checkouts), the legacy contract fallback and the initial-introduction detection would therefore fail instead of falling back.
The dispatcher binary depends on all seven common packages (verified via go list -deps), not just clicore, so changes to common/project or common/version could previously ship without the required dispatcherVersion bump.
The workspace files wire module resolution for the release build, so changing them can change the built binaries without touching any path the release-input detection previously watched.
RyanXie123
pushed a commit
to RyanXie123/unity-cli-loop
that referenced
this pull request
Sep 22, 2026
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
1 participant
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.
Summary
common/(shared packages),dispatcher/,project-runner/(renamed fromcli/), andtools/release-automation/, tied together by a repo-rootgo.work.uloop-project-runner-v3.0.0-beta.46) has been published and its stamps are merged into this branch.User Impact
uloopdispatcher anduloop-project-runnerbinaries are built from the same sources as before.common, so the compiler itself enforces the release boundary.Changes
common/(clicore, project, skills, tools, unityipc, version, clicontract),dispatcher/(with its contract and internal packages), andtools/release-automation/into their own modules; the remaining runner code moved fromcli/toproject-runner/with a matching module path.contract.json+ loader) now lives incommon/clicontract; the dispatcher contract lives indispatcher/. This deviates from the original plan (both under the runner) because the handshake and version stamps are needed by all three consumers.clirenamed toproject-runner; the componentuloop-project-runner, its tag series, and all stamped versions are unchanged.dispatcherVersionbumped to 3.0.1-beta.11 because dispatcher release inputs moved.dist/tree;.go-version, the golangci configs, and the layout contract (schema v2, declaring all four modules) moved to the repo root as single sources of truth. Workflows, packaging, sync, and release-target scripts follow.tools/release-automationand now validate the whole repository from the repo root: the pre-splitcli/directory must not reappear, module dependency directions are enforced from go.mod requires, the go directive stays aligned across go.mod/go.work/.go-version, internal boundary lists and the 500-line production file cap cover all four modules, and module enumeration is cross-checked againstgo.work, the check scripts, and the code-complexity workflow. Extending the file cap to all modules surfaced a 523-line guard file, which was resolved by extracting the contract-at-ref helpers into their own file.MINIMUM_REQUIRED_PROJECT_RUNNER_VERSIONconstant.Known transitional states
cli/contract.json/cli/dispatcher-contract.jsonpaths. This fallback is permanent because pre-split tags keep those paths forever.common/do not surface in release PRs (release-please package roots arePackages/srcandproject-runner).common/is treated as frozen in the interim.Verification
scripts/check-go-cli.sh: fmt / vet / lint / tests across all four modules, plus build and dist verification — green, including after merging theorigin/v3-betabeta.46 release stamps.cli/recreation, an illegalgo.modrequire, a clicore import insidecommon, an internal package incommon, and a layout-contract typo), then reverted.test-release-please-config,test-sync-release-please-package-releases,test-resolve-native-cli-release-target,test-native-cli-publish-workflow,test-go-cli-toolchain,test-install-release-filter,test-use-local-uloop,test-resolve-dispatcher-release-target.v2.1.1→v3.0.0-beta.48released tag) run against a scratch Unity project as the pre-merge gate.