chore: Record the dispatcher/runner split decision in a new docs/adr/ directory#1972
Merged
Conversation
Record why uloop keeps its two-binary CLI architecture instead of consolidating into a single self-contained binary like Unity's official CLI (unity 1.0.0-beta.2 + com.unity.pipeline, investigated 2026-07-24): a self-describing catalog carries schemas but not runner-side behavior, the per-project runner pin dissolves cross-project version skew that a single global binary would reintroduce, and the equality-gated IPC protocolVersion depends on package and runner releasing together. Follows the decision-record format of security-scan-engine-decision.md and states the reversal condition (force-latest package installs).
Give architecture decision records a dedicated home instead of mixing them with runbooks and guardrail docs in the flat docs/ root. Files are numbered in decision-date order (0001 security scan engine, 0002 dispatcher/runner split); no existing references needed updating.
Contributor
📝 WalkthroughWalkthroughTwo ADRs document the selected CodeQL-based C# security scanning approach and the decision to retain separate dispatcher and project-local runner binaries with pinned compatibility. ChangesC# Security Scanning
Dispatcher–Runner Architecture
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The force-latest manifest rewrite is avoidable by ordinary package discipline, so it is no longer presented as the structural objection. The refined objection is what remains once the package is pinned: aligning the CLI per project needs a recorded compatibility mapping, a mismatch gate, and per-project CLI selection — none of which the single-binary model provides, and all of which are exactly what the pin plus dispatcher implement. The latest-only install channel is explicitly excluded as a beta-stage limitation rather than a structural defect.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/adr/0002-dispatcher-runner-split.md (1)
15-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord the exact Unity Pipeline package version and evidence.
The comparison names
unity1.0.0-beta.2but leavescom.unity.pipelineunversioned, while the conclusions about/api/commands, manifest rewriting, and the absence of a protocol gate are version-sensitive. Add the package version and a reproducible artifact/source reference, or qualify these as observations from that specific build.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/0002-dispatcher-runner-split.md` around lines 15 - 21, Update the ADR’s Unity Pipeline comparison to identify the exact com.unity.pipeline package version and cite a reproducible artifact or source supporting the /api/commands behavior, manifest rewriting, and protocol-gate conclusion. If exact evidence cannot be added, explicitly qualify these statements as observations from the specific inspected build.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/adr/0002-dispatcher-runner-split.md`:
- Around line 15-21: Update the ADR’s Unity Pipeline comparison to identify the
exact com.unity.pipeline package version and cite a reproducible artifact or
source supporting the /api/commands behavior, manifest rewriting, and
protocol-gate conclusion. If exact evidence cannot be added, explicitly qualify
these statements as observations from the specific inspected build.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e4a9184-79e4-49ac-97e6-e052e3204009
📒 Files selected for processing (1)
docs/adr/0002-dispatcher-runner-split.md
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
docs/adr/directory with date-ordered numbering and moves the existing decision record into it.User Impact
Changes
docs/adr/0002-dispatcher-runner-split.md: compares against Unity's official CLI (single binary, self-describing/api/commandscatalog, force-latest package installs, investigated 2026-07-24) and documents why the split stays — a schema catalog cannot ship runner-side behavior, the runner pin dissolves cross-project version skew, and the equality-gated IPCprotocolVersiondepends on package and runner releasing together.docs/security-scan-engine-decision.mdtodocs/adr/0001-csharp-security-scan-engine.md(rename only, no content changes; no inbound references existed).Verification
git diff origin/v3-beta...HEAD --name-statusshows only the new ADR and a 100% rename.grep -rn "security-scan-engine-decision"across the repo confirmed no references needed updating.