diff --git a/AGENTS.md b/AGENTS.md index b8978d5934..b32e5052bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,58 +30,23 @@ Every test method must have a short comment that states what behavior the test v ## CLI / Unity Package Compatibility 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. - -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. - -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. -- 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. +protocol version. `protocolVersion` in `cli/common/clicontract/contract.json` and +`CliConstants.REQUIRED_CLI_PROTOCOL_VERSION` must always stay equal — never bump one alone, +and bump them (together, in the same PR) only when the IPC wire format becomes incompatible +between generations; ordinary features and fixes must not bump it. Release version fields +(`projectRunnerVersion`, `dispatcherVersion`, changelogs) are stamped by release-please only — +never edit them by hand in a feature PR (sole exception: the documented version-series +realignment; see `docs/version-series-realignment.md`). Bump criteria and release sequencing: +`docs/protocol-version.md`. ## 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. It -currently has two 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. - -There is no dispatcher⇄package integer contract generation; the pin's semver floor is the only -dispatcher gate. The IPC `protocolVersion` pair described above 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. +`Packages/src/project-runner-pin.json` (mirrored byte-identically to +`.uloop/project-runner-pin.json`) is the single source for cross-component version +requirements. `minimumDispatcherVersion` is the only manually maintained field — raise it only +when the package genuinely needs a newly published dispatcher. All other fields are stamped by +automation; never edit them by hand. The pin evolves additively only — never delete or rename +an existing field. Field reference and rationale: `docs/project-runner-pin.md`. ## Generated Skill Files @@ -98,101 +63,54 @@ Shell scripts are acceptable only as thin wrappers or simple command sequences. ## 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. - -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. +All three components release through release-please; `dispatcherVersion` and component +changelogs are stamped by release PRs — never bump them by hand (sole exception: the +documented version-series realignment; see `docs/version-series-realignment.md`). Changes to shared release +inputs outside the package roots (non-test `cli/common/**` sources, `scripts/install.sh`, +`scripts/install.ps1`) need matching trigger changes and a `scripts/stamp-release-inputs.sh` +run in the same PR; CI (`check-release-triggers`) fails otherwise. Rules and rationale: +`docs/shared-release-inputs.md`. ## 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. +Before changing scripts, skill files, generated-file synchronization, path handling, or text +parsing, read `docs/windows-compatibility.md` (encoding, line endings, path separators, +PowerShell validation). Add 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. ## 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 -``` - -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. +Before deleting apparently unreferenced C# code, or before adding comments explaining why an +apparently unreferenced type must stay, run the scanner and interpret its output conservatively +as described in `docs/dead-code-scanner.md` (commands, and what `KeptByUnityOrReflection` / +`PublicCandidate` do and do not prove). ## Native Go CLI Validation -When running `uloop` commands for this project during CLI development, do not use the `uloop` command resolved from `PATH`. Run this checkout's built development binary directly so validation uses the code under review: +When running `uloop` commands for this project during CLI development, do not use the `uloop` +resolved from `PATH`. Run this checkout's built development binary (rebuilt after relevant CLI +source changes) so validation uses the code under review: ```bash dist/darwin-arm64/uloop compile --project-path "$(git rev-parse --show-toplevel)" ``` -Before running a command with `--project-path`, confirm that the path is the intended Unity project for the current task. Do not copy a sibling checkout path from another repository or prior session. When intentionally validating a different Unity project, use an explicit placeholder in notes and replace it at execution time: - -```bash -dist/darwin-arm64/uloop compile --project-path -``` +Substitute the binary for your platform (e.g. `dist/windows-amd64/uloop.exe` on Windows). -If CLI source changes affect the command behavior you are validating, rebuild the development binary before running it. +Before running a command with `--project-path`, confirm the path is the intended Unity project +for the current task — do not copy a sibling checkout path from another repository or session. -When changing Go source files under any of the Go modules (`cli/common`, `cli/dispatcher`, `cli/project-runner`, `cli/release-automation`), run `scripts/check-go-cli.sh`. -Use `scripts/build-go-cli.sh` when you need to refresh local development binaries under `dist`; generated binaries are ignored and must not be committed. -This script is the local equivalent of the Go CLI CI validation: it runs formatting checks, vet, lint, tests, rebuilds the built native binaries, and verifies that required platform binaries exist. +When changing Go source files under any Go module (`cli/common`, `cli/dispatcher`, +`cli/project-runner`, `cli/release-automation`), run `scripts/check-go-cli.sh` — the local +equivalent of Go CLI CI (format, vet, lint, tests, binary rebuild). Use `scripts/build-go-cli.sh` +to refresh `dist` binaries; they are git-ignored and must not be committed. ## Unity Freeze Prevention -Do not add or keep Unity EditMode tests that can freeze the Editor. - -- 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. -- Treat these patterns as high risk in Unity EditMode and avoid them 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 -- 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. +Unity EditMode tests can freeze the Editor. Never run multiple `uloop run-tests` commands in +parallel — Unity Test Runner is single-flight only. Before adding or modifying Unity EditMode +tests (especially anything touching async execution, cancellation, threads, or dynamic-code +runtime paths), read `docs/unity-editmode-test-guardrails.md` and follow its rules. If a new +test makes `uloop run-tests` stall, remove or disable it instead of retrying the suite. If +Unity freezes or stops responding to `uloop`, restart the Editor with `uloop launch -r`. diff --git a/docs/dead-code-scanner.md b/docs/dead-code-scanner.md new file mode 100644 index 0000000000..2a146fd7db --- /dev/null +++ b/docs/dead-code-scanner.md @@ -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. diff --git a/docs/project-runner-pin.md b/docs/project-runner-pin.md new file mode 100644 index 0000000000..039fb77b14 --- /dev/null +++ b/docs/project-runner-pin.md @@ -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. diff --git a/docs/protocol-version.md b/docs/protocol-version.md new file mode 100644 index 0000000000..19c9422fbe --- /dev/null +++ b/docs/protocol-version.md @@ -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. diff --git a/docs/shared-release-inputs.md b/docs/shared-release-inputs.md new file mode 100644 index 0000000000..6f47609b30 --- /dev/null +++ b/docs/shared-release-inputs.md @@ -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. diff --git a/docs/unity-editmode-test-guardrails.md b/docs/unity-editmode-test-guardrails.md new file mode 100644 index 0000000000..19c3060c83 --- /dev/null +++ b/docs/unity-editmode-test-guardrails.md @@ -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. diff --git a/docs/version-series-realignment.md b/docs/version-series-realignment.md new file mode 100644 index 0000000000..57e1c3172c --- /dev/null +++ b/docs/version-series-realignment.md @@ -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`. + +## 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 + --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 `## []` 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. diff --git a/docs/windows-compatibility.md b/docs/windows-compatibility.md new file mode 100644 index 0000000000..407279045f --- /dev/null +++ b/docs/windows-compatibility.md @@ -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.