Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 45 additions & 127 deletions AGENTS.md

Large diffs are not rendered by default.

26 changes: 26 additions & 0 deletions docs/dead-code-scanner.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Dead code scanner

Use the C# dead-code scanner before deleting apparently unreferenced C# code or
before adding comments to explain why an apparently unreferenced type must stay.

For type-level review, especially when checking classes that may be kept by
Unity, serialization, reflection, release automation, or external package APIs,
run:

```bash
dotnet run --project tools/UnityCliLoop.DeadCodeScanner -- --scope public --include-types true --include-members false --include-locals false --include-test-only true --include-kept true --format table
```

For a broader member/local-variable pass, run:

```bash
dotnet run --project tools/UnityCliLoop.DeadCodeScanner -- --scope public --include-types true --include-members true --include-locals true --include-test-only true --include-kept false --format table
```

## Interpreting the output

Interpret scanner output conservatively:

- `KeptByUnityOrReflection` usually means the symbol is intentionally reachable through Unity callbacks, attributes, serialization, or reflection-style discovery. Do not add explanatory comments for every such symbol when the attribute/base type already makes the reason obvious.
- `PublicCandidate` means Roslyn found no direct references. Check non-C# references such as `release-please-config.json`, checked-in JSON contracts, Unity assets, generated files, and documented public APIs before removing or commenting the symbol.
- If a symbol is referenced only by non-C# tooling, verify that the tool reads it for runtime or release behavior. If the tool only rewrites the symbol and no code reads it, remove the marker instead of documenting it.
30 changes: 30 additions & 0 deletions docs/project-runner-pin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Project runner pin

`Packages/src/project-runner-pin.json` (mirrored byte-identically to
`.uloop/project-runner-pin.json` by `CliPinSynchronizer`) is the single source
for cross-component version requirements. Its required fields:

- `projectRunnerVersion` — the project runner release the dispatcher must run for this package.
Stamped by release-please; never edit by hand.
- `minimumDispatcherVersion` — the semver floor the package requires of the globally installed
dispatcher. The dispatcher force-updates itself when it is older than this value, and the
package reads it (via `CliPinReader`) for setup and installation checks. This is the only
manually maintained minimum-version declaration; raise it only when the package genuinely
needs a newly published dispatcher, not because the dispatcher implementation changed.
- `dispatcherReleaseTag` and `dispatcherArchiveManifest` — the provenance-pinned dispatcher
release used for first installation and its verified asset hashes. Stamped by automation
against a published release; never edit by hand — `VerifyDispatcherPinSubjects` requires the
manifest to match the published release's verified subjects exactly, so a hand-written value
cannot pass CI (see `docs/dispatcher-pin-release-order.md`).

There is no dispatcher⇄package integer contract generation; the pin's semver
floor is the only dispatcher gate. The IPC `protocolVersion` pair (see
`docs/protocol-version.md`) is the only integer generation in the system.

## Pin format discipline

The pin evolves additively only — never delete or rename an existing field.
The forced-update instruction (`minimumDispatcherVersion`) travels inside the
pin, so an old dispatcher that cannot parse a new pin never learns it must
update. For the same reason the dispatcher must stay lenient when reading pins
written by older packages.
40 changes: 40 additions & 0 deletions docs/protocol-version.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# CLI / Unity package protocol version

Runtime compatibility between the Unity package and the native CLI is gated on
an integer protocol version, not on release numbers. Two declarations must
always stay equal:

- Go side: `protocolVersion` in `cli/common/clicontract/contract.json` (the generation the CLI advertises over IPC).
- C# side: `CliConstants.REQUIRED_CLI_PROTOCOL_VERSION` (the exact generation the package accepts).

`TestProtocolVersionMatchesUnityPackage` fails the build if they diverge, so
never bump one alone. The runtime gate expects equality because the protocol
version is a contract generation, not a minimum-compatible range.

Pull request CI also runs a non-blocking IPC protocol reminder when IPC-facing
files changed without protocol declaration changes; treat it as a review
prompt, not as proof that a bump is required.

## When to bump

Bump both, together, in the same PR only when the IPC contract changes in a
way that makes CLI and package builds from different protocol generations
unable to interoperate — for example renaming or removing a request field,
changing the readiness/dispatch handshake, or altering a response shape the
other side parses. Ordinary CLI features and bug fixes that keep the wire
format compatible must not bump it.

## Release sequencing and mismatch guidance

Do not touch the protocol version to "keep up with releases":

- `cli/common/clicontract/contract.json` `projectRunnerVersion`, the pin files'
`projectRunnerVersion`, and `cli/dispatcher/dispatchercontract/dispatcher-contract.json`
`dispatcherVersion` are stamped by release-please only. Never edit them by hand in a feature PR
(the one exception is a version series realignment; see `docs/version-series-realignment.md`).
- When a protocol bump changes `CliConstants.REQUIRED_CLI_PROTOCOL_VERSION`, prepare the matching
project runner release first. PR CI (`check-protocol-minimum-version`) fails until the pin's
`projectRunnerVersion` points at a published project runner release that advertises the
required protocol; release-please advances that value when the runner release is cut.
- Runtime protocol mismatch guidance must use the unpinned CLI update path for older clients and
tell newer clients to align the package and CLI releases.
26 changes: 26 additions & 0 deletions docs/shared-release-inputs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Shared release inputs and triggers

The dispatcher is released through release-please like the project runner and
the Unity package: `cli/dispatcher/dispatchercontract/dispatcher-contract.json`
`dispatcherVersion` and `cli/dispatcher/CHANGELOG.md` are stamped by
release-please release PRs. Never bump `dispatcherVersion` by hand (the one
exception is a version series realignment; see
`docs/version-series-realignment.md`).

release-please attributes a commit to a component only when the commit touches
that package root (`Packages/src/`, `cli/dispatcher/`, `cli/project-runner/`).
Shared release inputs living outside those roots therefore need explicit
trigger updates in the same PR:

- Common module sources (non-test `cli/common/**/*.go`, `cli/common/go.mod`, `cli/common/go.sum`) must be
accompanied by changes under both `cli/project-runner/` and `cli/dispatcher/`.
- Installer scripts (`scripts/install.sh`, `scripts/install.ps1`) must be accompanied by a
change under `cli/dispatcher/`, because installers ship as dispatcher release assets.

Run `scripts/stamp-release-inputs.sh` to refresh
`cli/project-runner/shared-inputs-stamp.json` and
`cli/dispatcher/shared-inputs-stamp.json`, and commit the stamp updates with
the change. Pull request CI runs `check-release-triggers` (authoritative rules:
`releaseTriggerRules` in
`cli/release-automation/internal/automation/release_trigger_guard.go`) and
fails when shared release inputs changed without the matching triggers.
33 changes: 33 additions & 0 deletions docs/unity-editmode-test-guardrails.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Unity EditMode test guardrails

Do not add or keep Unity EditMode tests that can freeze the Editor. Read this
before adding or modifying Unity EditMode tests, especially anything touching
async execution, cancellation, threads, or dynamic-code runtime paths.

## Hard rules

- Never run multiple `uloop run-tests` commands in parallel. Treat Unity Test Runner as single-flight only.
- Do not add tests that rely on infinite waits, long-lived `TaskCompletionSource`, background fire-and-forget work, or cancellation handoff across domain reload boundaries.
- Avoid tests that intentionally cancel linked `CancellationTokenSource` instances while Unity may still dispose them during reload or teardown.
- Do not add Unity EditMode tests that use `Task.Run`, raw `Thread` work, or cross-thread coordination primitives such as `ManualResetEventSlim` unless the test is explicitly reviewed as unavoidable.
- Do not block the main thread inside Unity EditMode tests with `.Wait()`, `.Result`, `Task.WaitAll`, `Thread.Sleep`, or similar synchronous waiting APIs.
- Do not add Unity EditMode tests that execute real dynamic-code compile-and-run flows through `ExecuteDynamicCodeTool`, `DynamicCodeCompiler`, or similar end-to-end runtime paths when a pure unit test or compile-only test can cover the behavior.
- Do not add Unity EditMode tests that start nested test execution flows or any other long-running editor orchestration from inside a test body.

## High-risk patterns to avoid by default

- Disposing runtime objects while an async execution is still in flight
- Canceling an in-flight execution and then waiting for teardown on the same thread
- Tests that require editor-thread continuations while the test body is synchronously waiting
- Scheduling work onto background threads and then waiting for Unity main-thread continuations to complete
- Cross-thread registration/cancellation tests that depend on exact frame timing or teardown order
- Dynamic-code execution tests that compile code and then await timers, continuations, or runtime callbacks inside Unity EditMode
- Using `TaskCompletionSource` as a gate for execution/dispose races unless every completion path is guaranteed without Unity callbacks
- Assuming `[Timeout]` makes a test safe even when the runner itself can deadlock first

## Preferred approach and incident response

- Prefer pure unit tests for cancellation, dispose, and race-condition coverage. Only promote them to Unity EditMode after the logic is structured so the test completes without background leftovers or main-thread blocking.
- If a new test causes `uloop run-tests` to stall, immediately remove or disable that test instead of retrying the same suite repeatedly.
- If `Editor.log` shows messages such as `Attempted to call .Dispose on an already disposed CancellationTokenSource`, treat the latest cancellation-focused test changes as suspect first.
- If Unity freezes or stops responding to `uloop`, restart the Editor with `uloop launch -r` before attempting any further compile, test, or log commands.
74 changes: 74 additions & 0 deletions docs/version-series-realignment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Version series realignment

Use this document when a component's release-please version series must be
moved backward (for example, a component accidentally escaped the shared
`3.0.0-beta` line because a bootstrap release made release-please treat a
final version as shipped). The dispatcher realignment that shipped as
`dispatcher-v3.0.0-beta.19` (PR #1888) followed this procedure: an
unintended `dispatcher-v3.0.0` bootstrap release
had pushed the dispatcher onto a `3.1.0-beta` series, and it was rewound to
`3.0.0-beta.19`.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

## Mechanics that make a rewind possible

- release-please is manifest-driven. `.release-please-manifest.json` is the
version source of truth, and no CI guard forbids lowering a manifest entry.
- `scripts/install.sh` resolves `latest`/`latest-beta` by walking GitHub
releases in publish-date order, not by semver. A semver-lower release
published later is still selected as the newest, so new installs converge.
- Already-installed dispatchers converge through the periodic optional
self-update, which reinstalls the newest published release.

## Procedure

1. Delete the unintended release and its tag (`gh release delete <tag>
--cleanup-tag`). The "Protect release tags" ruleset blocks tag deletion for
everyone including admins; the repository owner must temporarily disable
the ruleset in the GitHub UI and re-enable it immediately afterwards.
Leave every other historical release in place — old package pins verify
their `minimumDispatcherVersion` against existing release tags.
2. In one commit, set the target version consistently across:
- the component's entry in `.release-please-manifest.json`,
- the component's version declaration (for the dispatcher,
`dispatchercontract/dispatcher-contract.json` — normally stamped by
release-please only; a realignment is the one legitimate manual edit),
- a matching `## [<version>]` heading at the top of the component's
`CHANGELOG.md`,
- both pin files' `minimumDispatcherVersion` when the dispatcher is
involved. The dispatcher minimum version guard passes without a release
lookup when the pin minimum equals the in-tree `dispatcherVersion`.
3. Merge the PR with an ordinary `fix:` title and let the publish automation
self-heal (see below). Do not create the missing release or tag by hand.

## How the missing release gets created

Two automations react differently to the realignment commit:

- `scripts/sync-release-please-package-releases.sh` only recognizes release
commits whose subject passes `scripts/is-release-please-release-commit.sh`
(`chore: release *` / `chore(...): release *`). A `fix:`-titled realignment
commit is invisible to it, so it cannot create the missing release and may
fail with "no release-please commit found" until the release exists.
- `.github/workflows/dispatcher-publish.yml`
(`scripts/resolve-dispatcher-release-target.sh`) ignores commit subjects
entirely. On every push it derives the release tag from the
`dispatcherVersion` in the contract at HEAD and, when that release is
missing or lacks assets, builds the binaries and creates/publishes the
release with attestations at the pushed commit. This is what materializes
the realigned version as a real release.

If the release-please workflow failed before dispatcher-publish finished,
re-run it via `workflow_dispatch` once the release exists with all assets.

## What not to touch

- Never hand-edit `dispatcherReleaseTag` or `dispatcherArchiveManifest` in the
pin files. `VerifyDispatcherPinSubjects` requires the pinned manifest to
match the published release's verified subjects exactly, so pointing them at
a not-yet-published tag cannot pass CI, and the archive hashes are unknowable
before the release exists. The pin stamp automation moves them on the next
normal dispatcher release (see `docs/dispatcher-pin-release-order.md`).
- Do not add an empty commit just to see the next version proposed. The
realignment commit itself becomes the tagged release commit, so release-please
correctly proposes nothing for the component until the next real change under
its package root.
14 changes: 14 additions & 0 deletions docs/windows-compatibility.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Windows compatibility guardrails

Most day-to-day development happens on macOS, but this project must keep
working on Windows. Before changing scripts, skill files, generated-file
synchronization, path handling, or text parsing, assume Windows will expose
bugs that macOS hides.

- Treat encoding as explicit input. When PowerShell reads UTF-8 repository files, pass `-Encoding UTF8`; Windows PowerShell 5.1 otherwise uses a legacy default that can corrupt non-ASCII text and even report wrong line numbers.
- Repository text files should use LF by default. Only keep CRLF when a specific tool or file format requires it. Preserve expected line endings when writing generated files, and normalize line endings before comparison only when logical text equality is intended. If a script fails only under bash, WSL, or Git Bash, check CRLF before changing logic.
- Normalize relative paths at API boundaries. Do not compare raw path strings that may contain `/` on one side and `\` on another. Convert separators before storing, comparing, deleting, or syncing generated files.
- Prefer forward slashes in JSON `file:` paths and other cross-platform config values. Use escaped backslashes only when the target format explicitly requires them.
- Validate Windows-facing PowerShell with both `pwsh` and Windows PowerShell when practical, especially for multiline arguments, here-strings, UTF-8 files, and native executable calls.
- When validating this checkout on Windows, use the repo-local native binary (`dist/windows-amd64/uloop.exe`) instead of a `PATH`-resolved `uloop`. If a bash validation command cannot see the expected Go toolchain on Windows, retry through a login shell such as `bash -lc`.
- Add or update a regression test whenever a fix depends on encoding, line endings, or separator normalization. A passing macOS test alone is not enough for these cases.