diff --git a/.agents/skills/uloop-hot-reload/references/output.md b/.agents/skills/uloop-hot-reload/references/output.md index ef63118d7..469359819 100644 --- a/.agents/skills/uloop-hot-reload/references/output.md +++ b/.agents/skills/uloop-hot-reload/references/output.md @@ -29,7 +29,7 @@ Returns JSON with: - `ActivePatchTotal` (number): Active changes after this run — patched methods plus added members. Introduced types are not counted here; `ActiveIntroducedTypeTotal` reports those. `--revert-all` clears the patched methods and added members counted here and reports their combined count in `ClearedCount`; introduced types stay loaded until the next Domain Reload and remain in `ActiveIntroducedTypeTotal`. Does not include `AddedField` rows. Validation failures (`HOT_RELOAD_NO_CHANGED_FILES` and the other `ErrorCode` cases) also report the live ledger value, not the default 0. - `AutoRefreshHeld` (boolean): True while Auto Refresh is held because at least one hot-reload change is still active. The first apply that arms the hold appends a Message sentence telling the caller to run `uloop compile` to release it, and that `--revert-all` releases it only when no introduced type remains — a revert cannot unload the assembly carrying an introduced type. A release during Play adds a Warning that pending script edits import on the next focus return or `uloop compile`. If the post-release Refresh is skipped because an open dirty scene also changed on disk, Warnings include the sentence telling the caller to resolve that scene and then run `uloop compile`. `--status` and `--revert-all` report the live value. - `AutoRefreshHoldMessage` (string, optional): The hold sentence this run appended to `Message` when it armed the hold; omitted when the run did not arm it. After a successful fallback compile, the CLI removes that sentence from `Message` and keeps this field as the record of what it removed. -- `Timing` (object, apply runs only): milliseconds this run spent per phase — `AnalysisMs` (transform worker runs, introduced-type preparation included), `ShimCompileMs` (signature gate and shim compile, isolation retries included), `PatchMs` (patch application), `OtherMs` (`TotalMs` minus the three phases: file resolution, planning, and the checks that find unchanged methods), `TotalMs` (the whole run inside the Editor). When the CLI ran the fallback compile, it adds `FallbackCompileMs`, the wall time of that compile, success or failure. Omitted on `--status` and `--revert-all`. +- `Timing` (object, apply runs only): milliseconds this run spent per phase — `AnalysisMs` (transform worker runs, introduced-type preparation included), `ShimCompileMs` (signature gate and shim compile, isolation retries included), `PatchMs` (patch application), `OtherMs` (`TotalMs` minus the three phases: file resolution, planning, and the checks that find unchanged methods), `TotalMs` (the whole run inside the Editor). When the CLI ran the fallback compile, it adds `FallbackCompileMs`, the wall time of that compile, success or failure. When it waited for the Editor to settle and applied again, it adds `EditorReadyWaitMs`, the wall time of that wait; the Editor's phases are then the second apply's. Omitted on `--status` and `--revert-all`. - `AddedFieldTotal` (number): Live added-field ledger rows after this run or on `--status`, added field-like events included. Those rows appear as `Kind` `AddedField` on `--status` only; they are not counted in `ActivePatchTotal` - `DroppedByPlayModeEntryCount` (number): Remaining patched-method, added-member, and introduced-type identities discarded by the Play-entry domain reload that have not been recovered by a later apply (`Patched` / `Added` methods, `Introduced` / `AlreadyActive` types — a recovered type also recovers the patches and added members inside it), `--revert-all`, or a successful compile. Omitted when the count is 0. Re-apply `uloop hot-reload`, or edit the files and run `uloop compile` - `RestoredWiredValueCount` (number, `--status` only): Values written through the added-field wiring call that the last scene reload (entering or leaving Play Mode with domain reload disabled) gave back to the rebuilt objects. It also counts the values `--status` itself gave back by reading them for a host that is back at its place. Omitted when 0. See [added-field-wiring.md](added-field-wiring.md) @@ -44,3 +44,4 @@ Returns JSON with: - `SelectedFiles` (string[], apply runs only): the project-relative asset paths of the scripts the run selected, whether given as `--files` or chosen as the changed files. The retry sends exactly these, as an explicit list, so a file the default selection would leave out is not left out on the second apply. Empty on `--status` and `--revert-all`. - `Compile` (object, present only when the CLI ran the fallback compile): the full `uloop compile` response; the top-level `Success` is then the compile's, and the command's exit code is the compile's. A successful compile drops `RecommendedNextAction`, sets `Outcome` to `ReplacedByCompile` and `AutoRefreshHeld` to `false`, removes the hold sentence from `Message`, and ends `Message` with a sentence saying the compile succeeded; a failed one leaves `Outcome`, `AutoRefreshHeld`, and `Message` as the reload reported them, and sets `RecommendedNextAction` to the compile's own `NextActions` when it reports any, and otherwise to fixing `Compile.Errors`. - `CompileFallbackNote` (string, present only with `Compile`): why the compile ran and how it ended. When it succeeded, `Outcome` is `ReplacedByCompile`, `AutoRefreshHeld` is `false`, and `Message` no longer carries the hold sentence. +- `EditorReadyRetryNote` (string, added by the CLI): present when `RetryAfterEditorReady` was `true`. It says the command waited for the Editor to settle and applied again, so every other field describes the second apply; or that the Editor did not settle within the wait, so the fields are the first apply's and no compile ran. A second apply that is refused again is not retried a third time: its `CompileFallback` decides the compile as usual. diff --git a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md index be4772af3..6c5b13ea1 100644 --- a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md @@ -431,6 +431,7 @@ source on disk. When a run skips a method it had patched before, `Warnings` name | Resolved assembly name is missing from CompilationPipeline | Per-file entry with `Method` = `(file)`; Unity may have mapped a not-yet-imported `.asmdef` onto a predefined assembly. Run `uloop compile` first | | Script is not in the last compiled assembly's source list and its assembly membership cannot be confirmed | Per-file entry with `Method` = `(file)`; a new file passed with `--files` is hot-reloadable when its membership in an existing, unchanged compiled assembly is confirmed (`.asmdef` / `.asmref` boundaries are checked when present; a predefined assembly with none also passes), but fails when the Editor is not ready or an `.asmdef` / `.asmref` on its path was added, deleted, or changed since the last import — run `uloop compile` first | | The Editor is a Multiplayer Play Mode Virtual Player | Per-file entry with `Method` = `(file)`; a Virtual Player has no compiled assemblies under its own project root, so hot reload cannot patch it yet. The edit reaches that player only through a compile: the CLI's compile fallback brings it in when `--compile-on-skip` lets the compile run, and `auto` holds it while that player is in Play Mode (`CompileFallback` is `HeldForPlayMode`). A patch applied to the main Editor does not reach Virtual Players (each is a separate Editor process) | +| The Editor is compiling or importing when the request arrives, or starts to before the reload is applied | Per-file entry with `Method` = `(file)`; nothing in the source needs a change. The response sets `RetryAfterEditorReady`, and the CLI waits for the Editor to settle (up to 10 minutes) and applies the same request again once; if Unity's own compile already took the edit in, the second apply reports `NothingToApply`. A second refusal falls through to `--compile-on-skip`. | | Loaded assembly differs from the one on disk (pending compile) | Run `uloop compile` first, then retry | | Source file fails to parse | Per-file `Failed` entry with `Method` = `(file)` carrying the parse errors; nothing from that file is applied, its earlier patches stay active, and `Success` is false | | Method signature not found in the loaded assembly | Usually a stale assembly; run `uloop compile`. In-file renames and signature changes are classified as added members before reaching this point | diff --git a/.claude/skills/uloop-hot-reload/references/output.md b/.claude/skills/uloop-hot-reload/references/output.md index ef63118d7..469359819 100644 --- a/.claude/skills/uloop-hot-reload/references/output.md +++ b/.claude/skills/uloop-hot-reload/references/output.md @@ -29,7 +29,7 @@ Returns JSON with: - `ActivePatchTotal` (number): Active changes after this run — patched methods plus added members. Introduced types are not counted here; `ActiveIntroducedTypeTotal` reports those. `--revert-all` clears the patched methods and added members counted here and reports their combined count in `ClearedCount`; introduced types stay loaded until the next Domain Reload and remain in `ActiveIntroducedTypeTotal`. Does not include `AddedField` rows. Validation failures (`HOT_RELOAD_NO_CHANGED_FILES` and the other `ErrorCode` cases) also report the live ledger value, not the default 0. - `AutoRefreshHeld` (boolean): True while Auto Refresh is held because at least one hot-reload change is still active. The first apply that arms the hold appends a Message sentence telling the caller to run `uloop compile` to release it, and that `--revert-all` releases it only when no introduced type remains — a revert cannot unload the assembly carrying an introduced type. A release during Play adds a Warning that pending script edits import on the next focus return or `uloop compile`. If the post-release Refresh is skipped because an open dirty scene also changed on disk, Warnings include the sentence telling the caller to resolve that scene and then run `uloop compile`. `--status` and `--revert-all` report the live value. - `AutoRefreshHoldMessage` (string, optional): The hold sentence this run appended to `Message` when it armed the hold; omitted when the run did not arm it. After a successful fallback compile, the CLI removes that sentence from `Message` and keeps this field as the record of what it removed. -- `Timing` (object, apply runs only): milliseconds this run spent per phase — `AnalysisMs` (transform worker runs, introduced-type preparation included), `ShimCompileMs` (signature gate and shim compile, isolation retries included), `PatchMs` (patch application), `OtherMs` (`TotalMs` minus the three phases: file resolution, planning, and the checks that find unchanged methods), `TotalMs` (the whole run inside the Editor). When the CLI ran the fallback compile, it adds `FallbackCompileMs`, the wall time of that compile, success or failure. Omitted on `--status` and `--revert-all`. +- `Timing` (object, apply runs only): milliseconds this run spent per phase — `AnalysisMs` (transform worker runs, introduced-type preparation included), `ShimCompileMs` (signature gate and shim compile, isolation retries included), `PatchMs` (patch application), `OtherMs` (`TotalMs` minus the three phases: file resolution, planning, and the checks that find unchanged methods), `TotalMs` (the whole run inside the Editor). When the CLI ran the fallback compile, it adds `FallbackCompileMs`, the wall time of that compile, success or failure. When it waited for the Editor to settle and applied again, it adds `EditorReadyWaitMs`, the wall time of that wait; the Editor's phases are then the second apply's. Omitted on `--status` and `--revert-all`. - `AddedFieldTotal` (number): Live added-field ledger rows after this run or on `--status`, added field-like events included. Those rows appear as `Kind` `AddedField` on `--status` only; they are not counted in `ActivePatchTotal` - `DroppedByPlayModeEntryCount` (number): Remaining patched-method, added-member, and introduced-type identities discarded by the Play-entry domain reload that have not been recovered by a later apply (`Patched` / `Added` methods, `Introduced` / `AlreadyActive` types — a recovered type also recovers the patches and added members inside it), `--revert-all`, or a successful compile. Omitted when the count is 0. Re-apply `uloop hot-reload`, or edit the files and run `uloop compile` - `RestoredWiredValueCount` (number, `--status` only): Values written through the added-field wiring call that the last scene reload (entering or leaving Play Mode with domain reload disabled) gave back to the rebuilt objects. It also counts the values `--status` itself gave back by reading them for a host that is back at its place. Omitted when 0. See [added-field-wiring.md](added-field-wiring.md) @@ -44,3 +44,4 @@ Returns JSON with: - `SelectedFiles` (string[], apply runs only): the project-relative asset paths of the scripts the run selected, whether given as `--files` or chosen as the changed files. The retry sends exactly these, as an explicit list, so a file the default selection would leave out is not left out on the second apply. Empty on `--status` and `--revert-all`. - `Compile` (object, present only when the CLI ran the fallback compile): the full `uloop compile` response; the top-level `Success` is then the compile's, and the command's exit code is the compile's. A successful compile drops `RecommendedNextAction`, sets `Outcome` to `ReplacedByCompile` and `AutoRefreshHeld` to `false`, removes the hold sentence from `Message`, and ends `Message` with a sentence saying the compile succeeded; a failed one leaves `Outcome`, `AutoRefreshHeld`, and `Message` as the reload reported them, and sets `RecommendedNextAction` to the compile's own `NextActions` when it reports any, and otherwise to fixing `Compile.Errors`. - `CompileFallbackNote` (string, present only with `Compile`): why the compile ran and how it ended. When it succeeded, `Outcome` is `ReplacedByCompile`, `AutoRefreshHeld` is `false`, and `Message` no longer carries the hold sentence. +- `EditorReadyRetryNote` (string, added by the CLI): present when `RetryAfterEditorReady` was `true`. It says the command waited for the Editor to settle and applied again, so every other field describes the second apply; or that the Editor did not settle within the wait, so the fields are the first apply's and no compile ran. A second apply that is refused again is not retried a third time: its `CompileFallback` decides the compile as usual. diff --git a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md index be4772af3..6c5b13ea1 100644 --- a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md @@ -431,6 +431,7 @@ source on disk. When a run skips a method it had patched before, `Warnings` name | Resolved assembly name is missing from CompilationPipeline | Per-file entry with `Method` = `(file)`; Unity may have mapped a not-yet-imported `.asmdef` onto a predefined assembly. Run `uloop compile` first | | Script is not in the last compiled assembly's source list and its assembly membership cannot be confirmed | Per-file entry with `Method` = `(file)`; a new file passed with `--files` is hot-reloadable when its membership in an existing, unchanged compiled assembly is confirmed (`.asmdef` / `.asmref` boundaries are checked when present; a predefined assembly with none also passes), but fails when the Editor is not ready or an `.asmdef` / `.asmref` on its path was added, deleted, or changed since the last import — run `uloop compile` first | | The Editor is a Multiplayer Play Mode Virtual Player | Per-file entry with `Method` = `(file)`; a Virtual Player has no compiled assemblies under its own project root, so hot reload cannot patch it yet. The edit reaches that player only through a compile: the CLI's compile fallback brings it in when `--compile-on-skip` lets the compile run, and `auto` holds it while that player is in Play Mode (`CompileFallback` is `HeldForPlayMode`). A patch applied to the main Editor does not reach Virtual Players (each is a separate Editor process) | +| The Editor is compiling or importing when the request arrives, or starts to before the reload is applied | Per-file entry with `Method` = `(file)`; nothing in the source needs a change. The response sets `RetryAfterEditorReady`, and the CLI waits for the Editor to settle (up to 10 minutes) and applies the same request again once; if Unity's own compile already took the edit in, the second apply reports `NothingToApply`. A second refusal falls through to `--compile-on-skip`. | | Loaded assembly differs from the one on disk (pending compile) | Run `uloop compile` first, then retry | | Source file fails to parse | Per-file `Failed` entry with `Method` = `(file)` carrying the parse errors; nothing from that file is applied, its earlier patches stay active, and `Success` is false | | Method signature not found in the loaded assembly | Usually a stale assembly; run `uloop compile`. In-file renames and signature changes are classified as added members before reaching this point | diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md index ef63118d7..469359819 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md @@ -29,7 +29,7 @@ Returns JSON with: - `ActivePatchTotal` (number): Active changes after this run — patched methods plus added members. Introduced types are not counted here; `ActiveIntroducedTypeTotal` reports those. `--revert-all` clears the patched methods and added members counted here and reports their combined count in `ClearedCount`; introduced types stay loaded until the next Domain Reload and remain in `ActiveIntroducedTypeTotal`. Does not include `AddedField` rows. Validation failures (`HOT_RELOAD_NO_CHANGED_FILES` and the other `ErrorCode` cases) also report the live ledger value, not the default 0. - `AutoRefreshHeld` (boolean): True while Auto Refresh is held because at least one hot-reload change is still active. The first apply that arms the hold appends a Message sentence telling the caller to run `uloop compile` to release it, and that `--revert-all` releases it only when no introduced type remains — a revert cannot unload the assembly carrying an introduced type. A release during Play adds a Warning that pending script edits import on the next focus return or `uloop compile`. If the post-release Refresh is skipped because an open dirty scene also changed on disk, Warnings include the sentence telling the caller to resolve that scene and then run `uloop compile`. `--status` and `--revert-all` report the live value. - `AutoRefreshHoldMessage` (string, optional): The hold sentence this run appended to `Message` when it armed the hold; omitted when the run did not arm it. After a successful fallback compile, the CLI removes that sentence from `Message` and keeps this field as the record of what it removed. -- `Timing` (object, apply runs only): milliseconds this run spent per phase — `AnalysisMs` (transform worker runs, introduced-type preparation included), `ShimCompileMs` (signature gate and shim compile, isolation retries included), `PatchMs` (patch application), `OtherMs` (`TotalMs` minus the three phases: file resolution, planning, and the checks that find unchanged methods), `TotalMs` (the whole run inside the Editor). When the CLI ran the fallback compile, it adds `FallbackCompileMs`, the wall time of that compile, success or failure. Omitted on `--status` and `--revert-all`. +- `Timing` (object, apply runs only): milliseconds this run spent per phase — `AnalysisMs` (transform worker runs, introduced-type preparation included), `ShimCompileMs` (signature gate and shim compile, isolation retries included), `PatchMs` (patch application), `OtherMs` (`TotalMs` minus the three phases: file resolution, planning, and the checks that find unchanged methods), `TotalMs` (the whole run inside the Editor). When the CLI ran the fallback compile, it adds `FallbackCompileMs`, the wall time of that compile, success or failure. When it waited for the Editor to settle and applied again, it adds `EditorReadyWaitMs`, the wall time of that wait; the Editor's phases are then the second apply's. Omitted on `--status` and `--revert-all`. - `AddedFieldTotal` (number): Live added-field ledger rows after this run or on `--status`, added field-like events included. Those rows appear as `Kind` `AddedField` on `--status` only; they are not counted in `ActivePatchTotal` - `DroppedByPlayModeEntryCount` (number): Remaining patched-method, added-member, and introduced-type identities discarded by the Play-entry domain reload that have not been recovered by a later apply (`Patched` / `Added` methods, `Introduced` / `AlreadyActive` types — a recovered type also recovers the patches and added members inside it), `--revert-all`, or a successful compile. Omitted when the count is 0. Re-apply `uloop hot-reload`, or edit the files and run `uloop compile` - `RestoredWiredValueCount` (number, `--status` only): Values written through the added-field wiring call that the last scene reload (entering or leaving Play Mode with domain reload disabled) gave back to the rebuilt objects. It also counts the values `--status` itself gave back by reading them for a host that is back at its place. Omitted when 0. See [added-field-wiring.md](added-field-wiring.md) @@ -44,3 +44,4 @@ Returns JSON with: - `SelectedFiles` (string[], apply runs only): the project-relative asset paths of the scripts the run selected, whether given as `--files` or chosen as the changed files. The retry sends exactly these, as an explicit list, so a file the default selection would leave out is not left out on the second apply. Empty on `--status` and `--revert-all`. - `Compile` (object, present only when the CLI ran the fallback compile): the full `uloop compile` response; the top-level `Success` is then the compile's, and the command's exit code is the compile's. A successful compile drops `RecommendedNextAction`, sets `Outcome` to `ReplacedByCompile` and `AutoRefreshHeld` to `false`, removes the hold sentence from `Message`, and ends `Message` with a sentence saying the compile succeeded; a failed one leaves `Outcome`, `AutoRefreshHeld`, and `Message` as the reload reported them, and sets `RecommendedNextAction` to the compile's own `NextActions` when it reports any, and otherwise to fixing `Compile.Errors`. - `CompileFallbackNote` (string, present only with `Compile`): why the compile ran and how it ended. When it succeeded, `Outcome` is `ReplacedByCompile`, `AutoRefreshHeld` is `false`, and `Message` no longer carries the hold sentence. +- `EditorReadyRetryNote` (string, added by the CLI): present when `RetryAfterEditorReady` was `true`. It says the command waited for the Editor to settle and applied again, so every other field describes the second apply; or that the Editor did not settle within the wait, so the fields are the first apply's and no compile ran. A second apply that is refused again is not retried a third time: its `CompileFallback` decides the compile as usual. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md index be4772af3..6c5b13ea1 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md @@ -431,6 +431,7 @@ source on disk. When a run skips a method it had patched before, `Warnings` name | Resolved assembly name is missing from CompilationPipeline | Per-file entry with `Method` = `(file)`; Unity may have mapped a not-yet-imported `.asmdef` onto a predefined assembly. Run `uloop compile` first | | Script is not in the last compiled assembly's source list and its assembly membership cannot be confirmed | Per-file entry with `Method` = `(file)`; a new file passed with `--files` is hot-reloadable when its membership in an existing, unchanged compiled assembly is confirmed (`.asmdef` / `.asmref` boundaries are checked when present; a predefined assembly with none also passes), but fails when the Editor is not ready or an `.asmdef` / `.asmref` on its path was added, deleted, or changed since the last import — run `uloop compile` first | | The Editor is a Multiplayer Play Mode Virtual Player | Per-file entry with `Method` = `(file)`; a Virtual Player has no compiled assemblies under its own project root, so hot reload cannot patch it yet. The edit reaches that player only through a compile: the CLI's compile fallback brings it in when `--compile-on-skip` lets the compile run, and `auto` holds it while that player is in Play Mode (`CompileFallback` is `HeldForPlayMode`). A patch applied to the main Editor does not reach Virtual Players (each is a separate Editor process) | +| The Editor is compiling or importing when the request arrives, or starts to before the reload is applied | Per-file entry with `Method` = `(file)`; nothing in the source needs a change. The response sets `RetryAfterEditorReady`, and the CLI waits for the Editor to settle (up to 10 minutes) and applies the same request again once; if Unity's own compile already took the edit in, the second apply reports `NothingToApply`. A second refusal falls through to `--compile-on-skip`. | | Loaded assembly differs from the one on disk (pending compile) | Run `uloop compile` first, then retry | | Source file fails to parse | Per-file `Failed` entry with `Method` = `(file)` carrying the parse errors; nothing from that file is applied, its earlier patches stay active, and `Success` is false | | Method signature not found in the loaded assembly | Usually a stale assembly; run `uloop compile`. In-file renames and signature changes are classified as added members before reaching this point | diff --git a/cli/project-runner/internal/projectrunner/hot_reload_compile_fallback.go b/cli/project-runner/internal/projectrunner/hot_reload_compile_fallback.go index 315ca8848..6ef27bafa 100644 --- a/cli/project-runner/internal/projectrunner/hot_reload_compile_fallback.go +++ b/cli/project-runner/internal/projectrunner/hot_reload_compile_fallback.go @@ -95,9 +95,14 @@ func runHotReloadWithCompileFallback( if len(result.result) == 0 { return result.exitCode } - // Why an ID of its own: the reload request's ID stays inside runPlainTool, so the fallback entries - // share this one and line up with the request entries by time. - correlationID := vibelog.NewCLIVibeCorrelationID() + retry := retryHotReloadAfterEditorReady(ctx, connection, params, stdout, stderr, result) + if retry.finished { + return retry.exitCode + } + result = retry.result + // Why the request's ID: the reader joins the fallback entries to the request's + // cli_tool_request_sent by it. + correlationID := result.correlationID requested := isHotReloadCompileFallbackRequested(result.result) logHotReloadCompileFallbackDecided(connection, correlationID, requested, result.result) if !requested { @@ -295,6 +300,11 @@ func injectHotReloadCompileFallback(raw json.RawMessage, compileRaw json.RawMess // Adds the fallback compile's wall time to Timing. The phases the Editor reported stay as they were, // and a response from an older package, which sends no Timing, gets one holding only the compile. func addHotReloadFallbackCompileTiming(raw []byte, elapsed time.Duration) ([]byte, error) { + return addHotReloadTimingMs(raw, hotReloadFallbackCompileMsField, elapsed) +} + +// addHotReloadTimingMs adds one CLI-side duration to Timing under name, in milliseconds. +func addHotReloadTimingMs(raw []byte, name string, elapsed time.Duration) ([]byte, error) { fields := map[string]json.RawMessage{} if err := json.Unmarshal(raw, &fields); err != nil { return nil, err @@ -307,7 +317,7 @@ func addHotReloadFallbackCompileTiming(raw []byte, elapsed time.Duration) ([]byt return nil, err } } - timing[hotReloadFallbackCompileMsField] = json.RawMessage(strconv.FormatInt(elapsed.Milliseconds(), 10)) + timing[name] = json.RawMessage(strconv.FormatInt(elapsed.Milliseconds(), 10)) encoded, err := json.Marshal(timing) if err != nil { return nil, err diff --git a/cli/project-runner/internal/projectrunner/hot_reload_compile_fallback_test.go b/cli/project-runner/internal/projectrunner/hot_reload_compile_fallback_test.go index 58762f0b6..eebc8a860 100644 --- a/cli/project-runner/internal/projectrunner/hot_reload_compile_fallback_test.go +++ b/cli/project-runner/internal/projectrunner/hot_reload_compile_fallback_test.go @@ -665,9 +665,11 @@ func TestRunHotReloadWritesFallbackDecidedAndCompleteVibeLogs(t *testing.T) { t.Fatalf("exit code = %d, want 0", code) } logContent := readOnlyCliVibeLog(t, projectRoot) - singleCliVibeEntry(t, logContent, "cli_tool_request_sent") + requestSent := singleCliVibeEntry(t, logContent, "cli_tool_request_sent") singleCliVibeEntry(t, logContent, "cli_tool_response_received") decided := singleCliVibeEntry(t, logContent, "cli_hot_reload_compile_fallback_decided") + // The reader joins the fallback entries to the request they follow by this ID. + assertSharedCliVibeCorrelationID(t, requestSent, decided) complete := singleCliVibeEntry(t, logContent, "cli_hot_reload_compile_fallback_complete") assertCliVibeEntryLevel(t, decided, "INFO") assertCliVibeEntryLevel(t, complete, "INFO") diff --git a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go new file mode 100644 index 000000000..5ad256d48 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go @@ -0,0 +1,340 @@ +package projectrunner + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "time" + + "github.com/hatayama/unity-cli-loop/common/clicontract" + "github.com/hatayama/unity-cli-loop/common/clicore" + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/vibelog" +) + +const ( + hotReloadRetryAfterEditorReadyField = "RetryAfterEditorReady" + hotReloadSelectedFilesField = "SelectedFiles" + hotReloadFilesParam = "Files" + hotReloadEditorReadyRetryNoteField = "EditorReadyRetryNote" + hotReloadEditorReadyWaitMsField = "EditorReadyWaitMs" +) + +const ( + hotReloadEditorReadyRetryDecidedOperation = "cli_hot_reload_editor_ready_retry_decided" + hotReloadEditorReadyRetryCompleteOperation = "cli_hot_reload_editor_ready_retry_complete" +) + +const ( + // %d is the whole seconds this command waited for the Editor. + hotReloadEditorReadyRetryNoteFormat = "The first apply was refused because the Editor was compiling or importing, so this command waited %ds for the Editor to settle and applied the same request again; every other field describes the second apply." + hotReloadEditorReadyGaveUpNoteFormat = "The apply was refused because the Editor was compiling or importing; this command waited %ds for the Editor to settle, but it did not, so no second apply ran." + hotReloadEditorReadyWaitingLine = "hot-reload: the Editor is compiling or importing, so the reload was refused; waiting for it to settle, then applying again..." +) + +type hotReloadEditorReadyWaitOptions struct { + pollInterval time.Duration + budget time.Duration + probeTimeout time.Duration +} + +// Why a package variable: the fallback tests already swap hotReloadFallbackCompile, so the tests +// of this file are not parallel either. +var hotReloadEditorReadyWaitDefaults = hotReloadEditorReadyWaitOptions{ + pollInterval: time.Second, + budget: compileWaitTimeout, + probeTimeout: 5 * time.Second, +} + +// hotReloadEditorReadyRetryOutcome is what the retry leaves for the fallback decision. +type hotReloadEditorReadyRetryOutcome struct { + // result is the response the fallback decision continues with. + result toolExecutionResult + // finished is true when stdout was already written and exitCode is the command's. + finished bool + exitCode int +} + +// retryHotReloadAfterEditorReady waits for the Editor to settle and applies the same files once +// more when the first response says it was refused only because the Editor was compiling or +// importing. Any other response passes through unchanged. +func retryHotReloadAfterEditorReady( + ctx context.Context, + connection unityipc.Connection, + params map[string]any, + stdout io.Writer, + stderr io.Writer, + first toolExecutionResult, +) hotReloadEditorReadyRetryOutcome { + // Why the first request's ID: the reader joins these entries to the request's + // cli_tool_request_sent by it. + correlationID := first.correlationID + requested, parseError := isHotReloadRetryAfterEditorReadyRequested(first.result) + logHotReloadEditorReadyRetryDecided(connection, correlationID, requested, parseError) + if !requested { + return hotReloadEditorReadyRetryOutcome{result: first} + } + + // A plain line rather than the tool spinner: the spinner is off on a non-TTY stderr, and on a + // TTY its Stop erases its own line, so the reader would not see why the command paused. + _, _ = fmt.Fprintln(stderr, hotReloadEditorReadyWaitingLine) + waited, ready, err := waitForEditorReady(ctx, connection, hotReloadEditorReadyWaitDefaults) + if err != nil { + clicore.WriteJSON(stdout, first.result) + writeHotReloadClassifiedError(stderr, connection, err) + logHotReloadEditorReadyRetryComplete(connection, correlationID, waited, false, nil) + return hotReloadEditorReadyRetryOutcome{finished: true, exitCode: 1} + } + if !ready { + return finishHotReloadEditorNeverReady(connection, stdout, stderr, first, waited) + } + return applyHotReloadAgain(ctx, connection, params, stdout, stderr, first, waited) +} + +// The first response stays the answer, with a note saying the Editor did not settle; no fallback +// compile runs, because the Editor that is still compiling would refuse it too. +func finishHotReloadEditorNeverReady( + connection unityipc.Connection, + stdout io.Writer, + stderr io.Writer, + first toolExecutionResult, + waited time.Duration, +) hotReloadEditorReadyRetryOutcome { + logHotReloadEditorReadyRetryComplete(connection, first.correlationID, waited, false, nil) + merged, err := injectHotReloadEditorReadyNote( + first.result, + fmt.Sprintf(hotReloadEditorReadyGaveUpNoteFormat, wholeSeconds(waited))) + if err != nil { + writeHotReloadClassifiedError(stderr, connection, err) + return hotReloadEditorReadyRetryOutcome{finished: true, exitCode: 1} + } + clicore.WriteJSON(stdout, merged) + return hotReloadEditorReadyRetryOutcome{finished: true, exitCode: first.exitCode} +} + +func applyHotReloadAgain( + ctx context.Context, + connection unityipc.Connection, + params map[string]any, + stdout io.Writer, + stderr io.Writer, + first toolExecutionResult, + waited time.Duration, +) hotReloadEditorReadyRetryOutcome { + // Why the selected files and not the same params: with no files given, the Editor selects the + // changed files anew, and after Unity's own compile took the edit in that selection is empty + // and fails validation; the files the first run named go through the explicit path and report + // NothingToApply instead. + retryParams := hotReloadRetryParams(params, hotReloadSelectedFiles(first.result)) + second := runPlainTool(ctx, connection, hotReloadCommandName, retryParams, stderr) + logHotReloadEditorReadyRetryComplete(connection, first.correlationID, waited, true, &second) + if len(second.result) == 0 { + // The transport failure is already classified on stderr; the first apply's answer is the + // last one the Editor gave. + clicore.WriteJSON(stdout, first.result) + return hotReloadEditorReadyRetryOutcome{finished: true, exitCode: second.exitCode} + } + + merged, err := injectHotReloadEditorReadyNote( + second.result, + fmt.Sprintf(hotReloadEditorReadyRetryNoteFormat, wholeSeconds(waited))) + if err == nil { + merged, err = addHotReloadTimingMs(merged, hotReloadEditorReadyWaitMsField, waited) + } + if err != nil { + writeHotReloadClassifiedError(stderr, connection, err) + return hotReloadEditorReadyRetryOutcome{finished: true, exitCode: 1} + } + return hotReloadEditorReadyRetryOutcome{result: toolExecutionResult{ + result: merged, + exitCode: second.exitCode, + correlationID: second.correlationID, + }} +} + +// Only an explicit true asks for the retry: a package that does not report the field never gets a +// second apply. parseError is true when the response is not a JSON object. +func isHotReloadRetryAfterEditorReadyRequested(raw []byte) (bool, bool) { + var fields map[string]json.RawMessage + // Why a nil map is a parse error too: JSON null decodes without an error and leaves it nil. + if err := json.Unmarshal(raw, &fields); err != nil || fields == nil { + return false, true + } + return string(fields[hotReloadRetryAfterEditorReadyField]) == "true", false +} + +// A missing or unreadable list counts as none, which leaves the request as it was sent. +func hotReloadSelectedFiles(raw []byte) []string { + answer := struct { + SelectedFiles []string `json:"SelectedFiles"` + }{} + if err := json.Unmarshal(raw, &answer); err != nil { + return nil + } + return answer.SelectedFiles +} + +// hotReloadRetryParams copies the first request and, when the first run named its files, sends +// exactly those. The caller's map is left as it was. +func hotReloadRetryParams(params map[string]any, selectedFiles []string) map[string]any { + retryParams := make(map[string]any, len(params)+1) + for key, value := range params { + retryParams[key] = value + } + if len(selectedFiles) > 0 { + retryParams[hotReloadFilesParam] = selectedFiles + } + return retryParams +} + +// waitForEditorReady polls the Editor status until the Editor answers and is ready, the budget +// runs out, or ctx is cancelled. +// Why every probe error counts as "not yet": the domain reload that follows Unity's compile takes +// the server down and brings it back. +func waitForEditorReady( + ctx context.Context, + connection unityipc.Connection, + options hotReloadEditorReadyWaitOptions, +) (time.Duration, bool, error) { + startedAt := time.Now() + deadline := startedAt.Add(options.budget) + for { + if probeEditorReady(ctx, connection, options.probeTimeout) { + return time.Since(startedAt), true, nil + } + if !time.Now().Before(deadline) { + return time.Since(startedAt), false, nil + } + select { + case <-ctx.Done(): + return time.Since(startedAt), false, ctx.Err() + case <-time.After(options.pollInterval): + } + } +} + +func probeEditorReady(ctx context.Context, connection unityipc.Connection, probeTimeout time.Duration) bool { + probeCtx, cancel := context.WithTimeout(ctx, probeTimeout) + defer cancel() + raw, err := unityipc.NewClient(connection, clicontract.ProjectRunnerVersion()).Send( + probeCtx, + editorStatusBridgeCommandName, + map[string]any{}) + if err != nil { + return false + } + var response editorStatusResponse + if err := json.Unmarshal(raw, &response); err != nil { + return false + } + return classifyEditorState(response) == statusStateReady +} + +func injectHotReloadEditorReadyNote(raw []byte, note string) ([]byte, error) { + fields := map[string]json.RawMessage{} + if err := json.Unmarshal(raw, &fields); err != nil { + return nil, err + } + if fields == nil { + return nil, errors.New("hot-reload response must be a JSON object") + } + encoded, err := json.Marshal(note) + if err != nil { + return nil, err + } + fields[hotReloadEditorReadyRetryNoteField] = encoded + return json.Marshal(fields) +} + +func wholeSeconds(duration time.Duration) int64 { + return int64(duration / time.Second) +} + +func writeHotReloadClassifiedError(stderr io.Writer, connection unityipc.Connection, err error) { + clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ + ProjectRoot: connection.ProjectRoot, + Command: hotReloadCommandName, + }) +} + +// Written whether or not the retry was requested, so the log shows a retry that did not run as +// well as one that did. +func logHotReloadEditorReadyRetryDecided( + connection unityipc.Connection, + correlationID string, + requested bool, + parseError bool, +) { + writePlainToolVibeLog(connection.ProjectRoot, func() vibelog.CLIVibeLogEntry { + return vibelog.CLIVibeLogEntry{ + Level: "INFO", + Operation: hotReloadEditorReadyRetryDecidedOperation, + Message: "Decided whether hot reload waits for the Editor and applies again.", + Context: map[string]any{ + "correlation_id": correlationID, + "requested": requested, + "parse_error": parseError, + }, + CorrelationID: correlationID, + } + }) +} + +// Written once on every way out of a retry that was requested. second is nil when no second +// apply was sent. +func logHotReloadEditorReadyRetryComplete( + connection unityipc.Connection, + correlationID string, + waited time.Duration, + ready bool, + second *toolExecutionResult, +) { + writePlainToolVibeLog(connection.ProjectRoot, func() vibelog.CLIVibeLogEntry { + entryContext := map[string]any{ + "correlation_id": correlationID, + "second_correlation_id": "", + "waited_ms": waited.Milliseconds(), + "ready": ready, + "second_result": false, + } + level := "WARN" + if second != nil { + entryContext["second_correlation_id"] = second.correlationID + addHotReloadSecondApplyContext(entryContext, second.result) + } + if ready && entryContext["second_result"] == true { + level = "INFO" + } + return vibelog.CLIVibeLogEntry{ + Level: level, + Operation: hotReloadEditorReadyRetryCompleteOperation, + Message: "Finished waiting for the Editor and applying hot reload again.", + Context: entryContext, + CorrelationID: correlationID, + } + }) +} + +// Reads only Success and Outcome: the rest of the response is text about the project's code, +// which the log never carries. +func addHotReloadSecondApplyContext(entryContext map[string]any, raw []byte) { + if len(raw) == 0 { + return + } + entryContext["second_result"] = true + var fields map[string]json.RawMessage + if err := json.Unmarshal(raw, &fields); err != nil || fields == nil { + return + } + switch string(fields[hotReloadSuccessField]) { + case "true": + entryContext["second_success"] = true + case "false": + entryContext["second_success"] = false + } + outcome, _, _ := readHotReloadStringField(fields, hotReloadOutcomeField) + entryContext["second_outcome"] = outcome +} diff --git a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go new file mode 100644 index 000000000..021e62c33 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go @@ -0,0 +1,663 @@ +package projectrunner + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net" + "reflect" + "strings" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +const ( + editorReadyRetryFirstRefused = `{"Success":false,"Outcome":"Failed","RetryAfterEditorReady":true,` + + `"SelectedFiles":["Assets/A.cs","Assets/B.cs"],"CompileFallback":"Requested","Message":"refused","Timing":{"TotalMs":12}}` + editorReadyRetryStatusCompiling = `{"IsBusy":false,"HasEditorState":true,"IsCompiling":true,"IsUpdating":false,` + + `"SecondsSinceLastMainThreadTick":0.1}` + editorReadyRetryStatusReady = `{"IsBusy":false,"HasEditorState":true,"IsCompiling":false,"IsUpdating":false,` + + `"SecondsSinceLastMainThreadTick":0.1}` + editorReadyRetrySecondApplied = `{"Success":true,"Outcome":"Applied","RetryAfterEditorReady":false,` + + `"CompileFallback":"NotNeeded","Message":"applied","Timing":{"TotalMs":34}}` +) + +// scriptedIPCStep is one request the fake Editor expects and how it answers it. +type scriptedIPCStep struct { + method string + result string + // drop closes the connection after reading the request, without an answer. + drop bool + // onServed runs after the answer was written, so a test can act at a known point of the run. + onServed func() +} + +type scriptedIPCRequest struct { + method string + params map[string]any +} + +// serveScriptedIPCResponses answers one connection per step in order, then reports any further +// request as unexpected, which is how a third apply is caught. +func serveScriptedIPCResponses( + listener net.Listener, + steps []scriptedIPCStep, + requests chan<- scriptedIPCRequest, + serverErr chan<- error, +) { + for index, step := range steps { + conn, err := listener.Accept() + if err != nil { + // Why silent: the listener closes at cleanup, and a run that ends early leaves the rest + // of the script unserved; the requests a test expects are checked on its own side. + return + } + request, err := readScriptedIPCRequest(conn) + if err != nil { + _ = conn.Close() + serverErr <- fmt.Errorf("step %d: %w", index, err) + return + } + requests <- request + if request.method != step.method { + _ = conn.Close() + serverErr <- fmt.Errorf("step %d: method mismatch: %s", index, request.method) + return + } + if step.drop { + _ = conn.Close() + continue + } + response := []byte(fmt.Sprintf(`{"jsonrpc":"2.0","result":%s,"id":1}`, step.result)) + writeErr := unityipc.Write(conn, response) + _ = conn.Close() + if writeErr != nil { + serverErr <- fmt.Errorf("step %d: %w", index, writeErr) + return + } + if step.onServed != nil { + step.onServed() + } + } + + conn, err := listener.Accept() + if err != nil { + return + } + defer func() { _ = conn.Close() }() + request, err := readScriptedIPCRequest(conn) + if err != nil { + return + } + serverErr <- fmt.Errorf("unexpected request after the script: %s", request.method) +} + +func readScriptedIPCRequest(conn net.Conn) (scriptedIPCRequest, error) { + payload, err := unityipc.Read(bufio.NewReader(conn)) + if err != nil { + return scriptedIPCRequest{}, err + } + request := struct { + Method string `json:"method"` + Params map[string]any `json:"params"` + }{} + if err := json.Unmarshal(payload, &request); err != nil { + return scriptedIPCRequest{}, err + } + return scriptedIPCRequest{method: request.Method, params: request.Params}, nil +} + +type editorReadyRetryRun struct { + stdout string + stderr string + code int + compileCalls int + requests []scriptedIPCRequest +} + +// hotReloadRequests keeps only the apply requests, in the order they arrived. +func (run editorReadyRetryRun) hotReloadRequests() []scriptedIPCRequest { + applies := []scriptedIPCRequest{} + for _, request := range run.requests { + if request.method == hotReloadCommandName { + applies = append(applies, request) + } + } + return applies +} + +func (run editorReadyRetryRun) stdoutFields(t *testing.T) map[string]any { + t.Helper() + fields := map[string]any{} + if err := json.Unmarshal([]byte(run.stdout), &fields); err != nil { + t.Fatalf("stdout is not a JSON object: %v\n%s", err, run.stdout) + } + return fields +} + +// useFastEditorReadyWait shortens the wait for the Editor so a scripted run finishes in +// milliseconds, and restores the defaults afterwards. +func useFastEditorReadyWait(t *testing.T, options hotReloadEditorReadyWaitOptions) { + t.Helper() + original := hotReloadEditorReadyWaitDefaults + hotReloadEditorReadyWaitDefaults = options + t.Cleanup(func() { + hotReloadEditorReadyWaitDefaults = original + }) +} + +func fastEditorReadyWaitOptions() hotReloadEditorReadyWaitOptions { + return hotReloadEditorReadyWaitOptions{ + pollInterval: time.Millisecond, + budget: 2 * time.Second, + probeTimeout: time.Second, + } +} + +// runScriptedHotReload runs hot reload against the scripted Editor, counting the fallback compiles +// the run asks for. The fallback compile answers with compileResult. +func runScriptedHotReload( + t *testing.T, + ctx context.Context, + projectRoot string, + params map[string]any, + steps []scriptedIPCStep, + compileResult compileExecutionResult, +) editorReadyRetryRun { + t.Helper() + listener := newLoopbackIpcListener(t) + requests := make(chan scriptedIPCRequest, len(steps)+1) + serverErr := make(chan error, len(steps)+1) + go serveScriptedIPCResponses(listener, steps, requests, serverErr) + + original := hotReloadFallbackCompile + compileCalls := 0 + hotReloadFallbackCompile = func(context.Context, unityipc.Connection, io.Writer) compileExecutionResult { + compileCalls++ + return compileResult + } + t.Cleanup(func() { + hotReloadFallbackCompile = original + }) + + connection := unityipc.Connection{ + Endpoint: unityipc.Endpoint{Network: listener.Addr().Network(), Address: listener.Addr().String()}, + ProjectRoot: projectRoot, + } + var stdout bytes.Buffer + var stderr bytes.Buffer + code := runHotReloadWithCompileFallback(ctx, connection, params, &stdout, &stderr) + + // Why a short grace: the post-script Accept reports a third apply asynchronously. + time.Sleep(20 * time.Millisecond) + assertServerDidNotFail(t, serverErr) + received := []scriptedIPCRequest{} + for { + select { + case request := <-requests: + received = append(received, request) + continue + default: + } + break + } + return editorReadyRetryRun{ + stdout: stdout.String(), + stderr: stderr.String(), + code: code, + compileCalls: compileCalls, + requests: received, + } +} + +func editorReadyRetryNoCompile() compileExecutionResult { + return compileExecutionResult{result: json.RawMessage(`{"Success":true}`), exitCode: 0} +} + +func withFiles(params map[string]any, files []any) map[string]any { + copied := map[string]any{} + for key, value := range params { + copied[key] = value + } + copied["Files"] = files + return copied +} + +func assertSecondApplyParams(t *testing.T, run editorReadyRetryRun, want func(first map[string]any) map[string]any) { + t.Helper() + applies := run.hotReloadRequests() + if len(applies) != 2 { + t.Fatalf("hot-reload requests = %d, want 2\nstdout=%s\nstderr=%s", len(applies), run.stdout, run.stderr) + } + expected := want(applies[0].params) + if !reflect.DeepEqual(applies[1].params, expected) { + t.Fatalf("second apply params = %#v, want %#v", applies[1].params, expected) + } +} + +func assertNoteAbsent(t *testing.T, fields map[string]any) { + t.Helper() + if _, present := fields["EditorReadyRetryNote"]; present { + t.Fatalf("EditorReadyRetryNote must be absent: %#v", fields["EditorReadyRetryNote"]) + } +} + +func assertAppliedAfterTheWait(t *testing.T, run editorReadyRetryRun) { + t.Helper() + if run.code != 0 { + t.Fatalf("exit code = %d, want 0\nstdout=%s\nstderr=%s", run.code, run.stdout, run.stderr) + } + if run.compileCalls != 0 { + t.Fatalf("compile calls = %d, want 0", run.compileCalls) + } + fields := run.stdoutFields(t) + if fields["Outcome"] != "Applied" { + t.Fatalf("Outcome = %#v, want Applied", fields["Outcome"]) + } + if note, _ := fields["EditorReadyRetryNote"].(string); note == "" { + t.Fatalf("EditorReadyRetryNote = %#v, want a sentence", fields["EditorReadyRetryNote"]) + } + timing, _ := fields["Timing"].(map[string]any) + if waited, isNumber := timing["EditorReadyWaitMs"].(float64); !isNumber || waited < 0 { + t.Fatalf("Timing.EditorReadyWaitMs = %#v, want a number of at least 0", timing["EditorReadyWaitMs"]) + } + if timing["TotalMs"] != float64(34) { + t.Fatalf("Timing.TotalMs = %#v, want the second apply's 34", timing["TotalMs"]) + } + if !strings.Contains(run.stderr, "waiting for it to settle") { + t.Fatalf("stderr must say the command is waiting: %q", run.stderr) + } +} + +// Verifies a refusal that only needs the Editor to settle is applied again once it is ready, with +// the files the first run selected, and the second apply's response is the one reported. +func TestRunHotReloadAppliesAgainOnceTheEditorIsReady(t *testing.T) { + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + run := runScriptedHotReload(t, context.Background(), t.TempDir(), + map[string]any{"CompileOnSkip": "auto"}, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusCompiling}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, result: editorReadyRetrySecondApplied}, + }, + editorReadyRetryNoCompile()) + + assertAppliedAfterTheWait(t, run) + assertSecondApplyParams(t, run, func(first map[string]any) map[string]any { + return withFiles(first, []any{"Assets/A.cs", "Assets/B.cs"}) + }) +} + +// Verifies the second apply resends the first request unchanged when the first response names no +// files, whether or not the caller gave files. +func TestRunHotReloadSendsTheSameParamsWhenTheFirstResponseNamesNoFiles(t *testing.T) { + cases := []struct { + name string + firstResponse string + params map[string]any + }{ + { + name: "absent", + firstResponse: `{"Success":false,"Outcome":"Failed","RetryAfterEditorReady":true,"CompileFallback":"Requested","Message":"refused"}`, + params: map[string]any{"CompileOnSkip": "auto"}, + }, + { + name: "empty", + firstResponse: `{"Success":false,"Outcome":"Failed","RetryAfterEditorReady":true,"SelectedFiles":[],"CompileFallback":"Requested","Message":"refused"}`, + params: map[string]any{"CompileOnSkip": "auto"}, + }, + { + name: "absent with given files", + firstResponse: `{"Success":false,"Outcome":"Failed","RetryAfterEditorReady":true,"CompileFallback":"Requested","Message":"refused"}`, + params: map[string]any{"CompileOnSkip": "auto", "Files": []any{"Assets/Given.cs"}}, + }, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + run := runScriptedHotReload(t, context.Background(), t.TempDir(), testCase.params, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: testCase.firstResponse}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, result: editorReadyRetrySecondApplied}, + }, + editorReadyRetryNoCompile()) + + assertSecondApplyParams(t, run, func(first map[string]any) map[string]any { + return first + }) + }) + } +} + +// Verifies the files the caller gave are replaced by the normalized list the first run selected. +func TestRunHotReloadReplacesGivenFilesWithTheSelectedOnes(t *testing.T) { + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + run := runScriptedHotReload(t, context.Background(), t.TempDir(), + map[string]any{"Files": []any{"./Assets/A.cs"}}, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: `{"Success":false,"Outcome":"Failed","RetryAfterEditorReady":true,` + + `"SelectedFiles":["Assets/A.cs"],"CompileFallback":"Requested","Message":"refused"}`}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, result: editorReadyRetrySecondApplied}, + }, + editorReadyRetryNoCompile()) + + assertSecondApplyParams(t, run, func(first map[string]any) map[string]any { + return withFiles(first, []any{"Assets/A.cs"}) + }) +} + +// Verifies a second apply that finds the edit already compiled in by Unity is reported as it is, +// with no compile. +func TestRunHotReloadKeepsASecondApplyThatHadNothingLeftToApply(t *testing.T) { + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + run := runScriptedHotReload(t, context.Background(), t.TempDir(), map[string]any{}, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, result: `{"Success":true,"Outcome":"NothingToApply","RetryAfterEditorReady":false,` + + `"CompileFallback":"NotNeeded","Message":"nothing"}`}, + }, + editorReadyRetryNoCompile()) + + if run.code != 0 || run.compileCalls != 0 { + t.Fatalf("exit code = %d, compile calls = %d, want 0 and 0\nstderr=%s", run.code, run.compileCalls, run.stderr) + } + if outcome := run.stdoutFields(t)["Outcome"]; outcome != "NothingToApply" { + t.Fatalf("Outcome = %#v, want NothingToApply", outcome) + } +} + +// Verifies the wait survives the Editor dropping the connection while it reloads its domain. +func TestRunHotReloadKeepsWaitingWhileTheEditorRestarts(t *testing.T) { + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + run := runScriptedHotReload(t, context.Background(), t.TempDir(), map[string]any{}, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusCompiling}, + {method: editorStatusBridgeCommandName, drop: true}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, result: editorReadyRetrySecondApplied}, + }, + editorReadyRetryNoCompile()) + + assertAppliedAfterTheWait(t, run) +} + +// Verifies a second refusal is not applied a third time; the second response decides the fallback +// compile, and both notes are reported. +func TestRunHotReloadDoesNotApplyAThirdTimeAndLetsTheSecondResponseDecideTheCompile(t *testing.T) { + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + run := runScriptedHotReload(t, context.Background(), t.TempDir(), map[string]any{}, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + }, + compileExecutionResult{result: json.RawMessage(`{"Success":true}`), exitCode: 0}) + + if applies := len(run.hotReloadRequests()); applies != 2 { + t.Fatalf("hot-reload requests = %d, want 2", applies) + } + if run.compileCalls != 1 { + t.Fatalf("compile calls = %d, want 1", run.compileCalls) + } + if run.code != 0 { + t.Fatalf("exit code = %d, want the compile's 0\nstderr=%s", run.code, run.stderr) + } + fields := run.stdoutFields(t) + for _, name := range []string{"Compile", "CompileFallbackNote", "EditorReadyRetryNote"} { + if _, present := fields[name]; !present { + t.Fatalf("%s missing from stdout: %s", name, run.stdout) + } + } +} + +// Verifies a response that does not ask for the retry is applied once, as before. +func TestRunHotReloadDoesNotApplyAgainUnlessAsked(t *testing.T) { + cases := map[string]string{ + "absent": `{"Success":true,"Outcome":"Applied","CompileFallback":"NotNeeded","Message":"applied"}`, + "false": `{"Success":true,"Outcome":"Applied","RetryAfterEditorReady":false,"CompileFallback":"NotNeeded","Message":"applied"}`, + } + for name, response := range cases { + t.Run(name, func(t *testing.T) { + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + run := runScriptedHotReload(t, context.Background(), t.TempDir(), map[string]any{}, + []scriptedIPCStep{{method: hotReloadCommandName, result: response}}, + editorReadyRetryNoCompile()) + + if len(run.requests) != 1 || run.compileCalls != 0 || run.code != 0 { + t.Fatalf("requests = %d, compile calls = %d, exit code = %d, want 1, 0, 0", + len(run.requests), run.compileCalls, run.code) + } + assertNoteAbsent(t, run.stdoutFields(t)) + }) + } +} + +// Verifies an Editor that does not settle within the budget leaves the first response with a note +// saying so, and neither a second apply nor a compile runs. +func TestRunHotReloadGivesUpWhenTheEditorDoesNotSettle(t *testing.T) { + options := fastEditorReadyWaitOptions() + options.budget = 30 * time.Millisecond + useFastEditorReadyWait(t, options) + steps := []scriptedIPCStep{{method: hotReloadCommandName, result: editorReadyRetryFirstRefused}} + for range 50 { + steps = append(steps, scriptedIPCStep{method: editorStatusBridgeCommandName, result: editorReadyRetryStatusCompiling}) + } + run := runScriptedHotReload(t, context.Background(), t.TempDir(), map[string]any{}, steps, editorReadyRetryNoCompile()) + + if applies := len(run.hotReloadRequests()); applies != 1 { + t.Fatalf("hot-reload requests = %d, want 1", applies) + } + if run.compileCalls != 0 || run.code != 1 { + t.Fatalf("compile calls = %d, exit code = %d, want 0 and 1", run.compileCalls, run.code) + } + fields := run.stdoutFields(t) + if fields["Outcome"] != "Failed" { + t.Fatalf("Outcome = %#v, want Failed", fields["Outcome"]) + } + if note, _ := fields["EditorReadyRetryNote"].(string); !strings.Contains(note, "did not") { + t.Fatalf("EditorReadyRetryNote = %#v, want the gave-up sentence", fields["EditorReadyRetryNote"]) + } +} + +// Verifies a cancel during the wait reports the cancel and leaves the first response as it was. +func TestRunHotReloadReportsACancelWhileWaitingForTheEditor(t *testing.T) { + options := fastEditorReadyWaitOptions() + options.pollInterval = 50 * time.Millisecond + useFastEditorReadyWait(t, options) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + steps := []scriptedIPCStep{ + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusCompiling, onServed: cancel}, + } + for range 50 { + steps = append(steps, scriptedIPCStep{method: editorStatusBridgeCommandName, result: editorReadyRetryStatusCompiling}) + } + run := runScriptedHotReload(t, ctx, t.TempDir(), map[string]any{}, steps, editorReadyRetryNoCompile()) + + if run.code != 1 || run.compileCalls != 0 { + t.Fatalf("exit code = %d, compile calls = %d, want 1 and 0", run.code, run.compileCalls) + } + if !strings.Contains(run.stderr, context.Canceled.Error()) { + t.Fatalf("stderr must report the cancel: %q", run.stderr) + } + fields := run.stdoutFields(t) + if fields["Outcome"] != "Failed" { + t.Fatalf("Outcome = %#v, want the first response's Failed", fields["Outcome"]) + } + assertNoteAbsent(t, fields) +} + +// Verifies a second apply that gets no answer leaves the first response on stdout and fails the +// command with the transport error. +func TestRunHotReloadKeepsTheFirstResponseWhenTheSecondRequestFails(t *testing.T) { + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + run := runScriptedHotReload(t, context.Background(), t.TempDir(), map[string]any{}, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, drop: true}, + }, + editorReadyRetryNoCompile()) + + if run.code != 1 || run.compileCalls != 0 { + t.Fatalf("exit code = %d, compile calls = %d, want 1 and 0", run.code, run.compileCalls) + } + // Why the error code and not any text: the waiting line is on stderr whenever the retry runs. + if !strings.Contains(run.stderr, "UNITY_DISCONNECTED_AFTER_DISPATCH") { + t.Fatalf("stderr must carry the transport error: %q", run.stderr) + } + fields := run.stdoutFields(t) + if fields["Message"] != "refused" { + t.Fatalf("Message = %#v, want the first response's", fields["Message"]) + } + assertNoteAbsent(t, fields) +} + +// Verifies the retry writes its decision and its end to the CLI vibe log under the first request's +// correlation ID, and names the second request's ID. +func TestRunHotReloadWritesEditorReadyRetryVibeLogs(t *testing.T) { + enableCliVibeLog(t) + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + projectRoot := t.TempDir() + run := runScriptedHotReload(t, context.Background(), projectRoot, map[string]any{}, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, result: editorReadyRetrySecondApplied}, + }, + editorReadyRetryNoCompile()) + if run.code != 0 { + t.Fatalf("exit code = %d, want 0\nstderr=%s", run.code, run.stderr) + } + + logContent := readOnlyCliVibeLog(t, projectRoot) + sent := cliVibeEntriesForOperation(t, logContent, "cli_tool_request_sent") + if len(sent) != 2 { + t.Fatalf("cli_tool_request_sent entries = %d, want 2\n%s", len(sent), logContent) + } + decided := singleCliVibeEntry(t, logContent, "cli_hot_reload_editor_ready_retry_decided") + complete := singleCliVibeEntry(t, logContent, "cli_hot_reload_editor_ready_retry_complete") + assertCliVibeContextValues(t, cliVibeEntryContext(t, decided), map[string]any{"requested": true}) + completeContext := cliVibeEntryContext(t, complete) + assertCliVibeContextValues(t, completeContext, map[string]any{ + "ready": true, + "second_success": true, + "second_outcome": "Applied", + }) + if _, isNumber := completeContext["waited_ms"].(float64); !isNumber { + t.Fatalf("waited_ms = %#v, want a number", completeContext["waited_ms"]) + } + assertSharedCliVibeCorrelationID(t, sent[0], decided) + assertSharedCliVibeCorrelationID(t, decided, complete) + second := vibeLogContextString(t, sent[1], "correlation_id") + if completeContext["second_correlation_id"] != second { + t.Fatalf("second_correlation_id = %#v, want the second request's %q", completeContext["second_correlation_id"], second) + } + // The fallback decision reads the second answer, so its entry follows the second request. + fallbackDecided := singleCliVibeEntry(t, logContent, "cli_hot_reload_compile_fallback_decided") + assertSharedCliVibeCorrelationID(t, sent[1], fallbackDecided) +} + +// Verifies a response that does not ask for the retry, or cannot be read, logs the decision and no +// end entry. +func TestRunHotReloadLogsAnEditorReadyRetryThatWasNotAskedFor(t *testing.T) { + cases := map[string]struct { + response string + parseError bool + }{ + "absent": {response: `{"Success":true,"Outcome":"Applied","CompileFallback":"NotNeeded","Message":"applied"}`}, + "null": {response: `null`, parseError: true}, + } + for name, testCase := range cases { + t.Run(name, func(t *testing.T) { + enableCliVibeLog(t) + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + projectRoot := t.TempDir() + runScriptedHotReload(t, context.Background(), projectRoot, map[string]any{}, + []scriptedIPCStep{{method: hotReloadCommandName, result: testCase.response}}, + editorReadyRetryNoCompile()) + + logContent := readOnlyCliVibeLog(t, projectRoot) + decided := singleCliVibeEntry(t, logContent, "cli_hot_reload_editor_ready_retry_decided") + assertCliVibeContextValues(t, cliVibeEntryContext(t, decided), map[string]any{ + "requested": false, + "parse_error": testCase.parseError, + }) + assertNoCliVibeEntry(t, logContent, "cli_hot_reload_editor_ready_retry_complete") + }) + } +} + +// Verifies a second response that is not a JSON object fails the command instead of being written +// without the note that says it is the second apply. +func TestRunHotReloadFailsWhenTheSecondResponseIsNotAnObject(t *testing.T) { + enableCliVibeLog(t) + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + projectRoot := t.TempDir() + run := runScriptedHotReload(t, context.Background(), projectRoot, map[string]any{}, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, result: `null`}, + }, + editorReadyRetryNoCompile()) + + if run.code != 1 || run.compileCalls != 0 { + t.Fatalf("exit code = %d, compile calls = %d, want 1 and 0", run.code, run.compileCalls) + } + if run.stdout != "" || !strings.Contains(run.stderr, "must be a JSON object") { + t.Fatalf("stdout must be empty and stderr must carry the error: stdout=%q stderr=%q", run.stdout, run.stderr) + } + complete := singleCliVibeEntry(t, readOnlyCliVibeLog(t, projectRoot), "cli_hot_reload_editor_ready_retry_complete") + completeContext := cliVibeEntryContext(t, complete) + assertCliVibeContextValues(t, completeContext, map[string]any{"second_result": true}) + assertCliVibeContextOmits(t, completeContext, "second_success", "second_outcome") +} + +// Verifies the second apply's Success is logged as false when the Editor refused it again. +func TestRunHotReloadLogsASecondRefusalAsUnsuccessful(t *testing.T) { + enableCliVibeLog(t) + useFastEditorReadyWait(t, fastEditorReadyWaitOptions()) + projectRoot := t.TempDir() + runScriptedHotReload(t, context.Background(), projectRoot, map[string]any{}, + []scriptedIPCStep{ + {method: hotReloadCommandName, result: editorReadyRetryFirstRefused}, + {method: editorStatusBridgeCommandName, result: editorReadyRetryStatusReady}, + {method: hotReloadCommandName, result: `{"Success":false,"Outcome":"Failed","CompileFallback":"NotNeeded","Message":"refused"}`}, + }, + editorReadyRetryNoCompile()) + + complete := singleCliVibeEntry(t, readOnlyCliVibeLog(t, projectRoot), "cli_hot_reload_editor_ready_retry_complete") + assertCliVibeContextValues(t, cliVibeEntryContext(t, complete), map[string]any{ + "second_success": false, + "second_outcome": "Failed", + }) +} + +// Verifies only a JSON object takes the note: anything else is reported as an error. +func TestInjectHotReloadEditorReadyNoteRejectsANonObject(t *testing.T) { + for _, raw := range []string{`null`, `[1]`, `not json`} { + if _, err := injectHotReloadEditorReadyNote([]byte(raw), "note"); err == nil { + t.Fatalf("injectHotReloadEditorReadyNote(%s) succeeded, want an error", raw) + } + } +} + +// Verifies a SelectedFiles value that is not a list of strings counts as no list, so the request +// is resent as it was. +func TestHotReloadSelectedFilesTreatsAnUnreadableListAsNone(t *testing.T) { + if files := hotReloadSelectedFiles([]byte(`{"SelectedFiles":"Assets/A.cs"}`)); files != nil { + t.Fatalf("hotReloadSelectedFiles = %#v, want nil", files) + } +} diff --git a/cli/project-runner/internal/projectrunner/run.go b/cli/project-runner/internal/projectrunner/run.go index 2b618fc17..1b12580ec 100644 --- a/cli/project-runner/internal/projectrunner/run.go +++ b/cli/project-runner/internal/projectrunner/run.go @@ -106,6 +106,9 @@ func runToolExecution( type toolExecutionResult struct { result json.RawMessage exitCode int + // correlationID is the vibe log ID of the request that produced this result, so entries + // written after it can be joined to that request. + correlationID string } func runPlainTool(ctx context.Context, connection unityipc.Connection, command string, params map[string]any, stderr io.Writer) toolExecutionResult { @@ -129,13 +132,13 @@ func runPlainTool(ctx context.Context, connection unityipc.Connection, command s ProjectRoot: connection.ProjectRoot, Command: command, }) - return toolExecutionResult{exitCode: 1} + return toolExecutionResult{exitCode: 1, correlationID: correlationID} } result := stripDebugTimingResult(command, outcome.Result) writeDebugTiming(stderr, command, time.Since(startedAt), outcome) exitCode := toolEnvelopeExitCode(result) logPlainToolResponseReceived(connection, command, correlationID, time.Since(startedAt), outcome, result, exitCode) - return toolExecutionResult{result: result, exitCode: exitCode} + return toolExecutionResult{result: result, exitCode: exitCode, correlationID: correlationID} } func runCompileWithDomainReloadWait(ctx context.Context, connection unityipc.Connection, params map[string]any, stdout io.Writer, stderr io.Writer) int { diff --git a/docs/vibe-logs.md b/docs/vibe-logs.md index d0b299ed7..3ebc931fe 100644 --- a/docs/vibe-logs.md +++ b/docs/vibe-logs.md @@ -68,7 +68,9 @@ or `control-play-mode` that waits for a domain reload or a Play Mode change, and | `cli_tool_request_sent` | before the request is sent | `command`, `correlation_id`, `project_identity`, `cli_version`, `param_keys` (sorted), `array_lengths` (element count of each array parameter) | | `cli_tool_response_received` | when Unity answered | `command`, `correlation_id`, `elapsed_ms`, `request_accepted`, `result_bytes`, `exit_code` | | `cli_tool_request_failed` (`ERROR`) | when no answer came, or Unity answered with an error | `command`, `correlation_id`, `elapsed_ms`, `request_accepted`, `error_kind` (`rpc:`, `final_response_timeout`, or `other`) | -| `cli_hot_reload_compile_fallback_decided` | after every `hot-reload` answer | `correlation_id`, `requested`, `parse_error`, and the answer's `success`, `outcome`, `warnings_count`, and `timing` (numbers only) | +| `cli_hot_reload_editor_ready_retry_decided` | after every `hot-reload` answer, before the fallback decision | `correlation_id` (the request's), `requested`, `parse_error` | +| `cli_hot_reload_editor_ready_retry_complete` (`WARN` unless the Editor settled and the second apply answered) | after the wait and the second apply, when a retry was requested | `correlation_id` (the first request's), `second_correlation_id` (the second request's, or empty when none was sent), `waited_ms`, `ready`, `second_result`, and the second answer's `second_success` and `second_outcome` | +| `cli_hot_reload_compile_fallback_decided` | after every `hot-reload` answer the fallback decision sees: the second one after an editor-ready retry, and none when that retry ended the command (see `cli_hot_reload_editor_ready_retry_complete`) | `correlation_id` (the request's), `requested`, `parse_error`, and the answer's `success`, `outcome`, `warnings_count`, and `timing` (numbers only) | | `cli_hot_reload_compile_fallback_complete` (`ERROR` unless `succeeded`) | after the fallback compile, when one ran | `correlation_id`, `elapsed_ms`, `compile_exit_code`, `compile_result_bytes`, `merged`, `succeeded` | - These entries hold keys, counts, sizes, and flags, never a parameter value, a response body,