fix: Pause point timeout details carry the wait's timeout and the marker id, and busy vibe entries name the running tool and the resent requests

[hatayama]

Summary

• A timed-out await-pause-point now reports its own --timeout-seconds as Details.TimeoutSeconds, the same value its message quotes. The marker's window moves to a new Details.MarkerTimeoutSeconds.
• Details.Id of the same failure now carries the marker id as the Editor normalized it (Assets/Foo.cs:42), matching the status command, instead of the form that was typed (./Assets/Foo.cs:42).
• The CLI vibe log's busy-wait complete entry lists the correlation id of every request sent again during the wait (resend_correlation_ids), so those requests can be traced from it.
• A busy cli_tool_request_failed entry names the tool that held the Editor (running_tool_name).

Behaviour change

uloop await-pause-point --id ./Assets/Foo.cs:42 --timeout-seconds 5 against a marker enabled with a 30-second window, after the wait times out:

• Before: {"Id":"./Assets/Foo.cs:42","TimeoutSeconds":30,...} with the message "Pause point was not hit within 5s."
• After: {"Id":"Assets/Foo.cs:42","TimeoutSeconds":5,"MarkerTimeoutSeconds":30,...} with the same message.

Status answer	Path	Before	After
Present	await alone (wait 5 s, marker 30 s)	TimeoutSeconds 30, Id as typed	TimeoutSeconds 5, MarkerTimeoutSeconds 30, Id the marker id
Missing	await alone	TimeoutSeconds 5, Id as typed	Unchanged; MarkerTimeoutSeconds is omitted rather than 0
Present	enable + await (both 30 s)	TimeoutSeconds 30, Id as typed	TimeoutSeconds 30, MarkerTimeoutSeconds 30, Id the marker id

The meaning is the same for timed-out, cleared and expired waits: TimeoutSeconds is the wait's, MarkerTimeoutSeconds the marker's.

Log input	Before	After
Busy answer whose data names the running tool	error_kind only	adds running_tool_name
Busy answer with no name, or a non-busy RPC error	error_kind only	unchanged (no key)
Busy wait with no resend	resends 0	adds resend_correlation_ids: []
Busy wait with two resends, the second getting in	resends 2	adds both ids in order; the last equals second_correlation_id

Changes

• The pause point failure details take TimeoutSeconds from the wait options, add MarkerTimeoutSeconds only when the status answer reported a window, and prefer the status answer's marker id, falling back to the typed id.
• An existing test that pinned the marker window under TimeoutSeconds for an expired marker now checks it under MarkerTimeoutSeconds, and checks that TimeoutSeconds is the wait's.
• One reader for the busy answer's running tool name is shared by the failure log entry and the hot-reload busy note.
• The busy wait records each resent request's correlation id; the complete entry writes them as an array that is never null.
• docs/vibe-logs.md describes both new keys.

Verification

Run in cli/project-runner:

• gofmt -l . — no output; go vet ./... — clean; golangci-lint run ./... — 0 issues; golangci-lint run -c ../.golangci-complexity.yml ./... — 0 issues.
• go test ./... -count=1 — everything passes except TestSendWithTransientConnectionRetryAbortsOnRefusedConnect, which cannot bind a Unix socket inside the sandboxed shell used for this change and does not touch this code. With that one test skipped, the module passes.
• Coverage (/cmd/ excluded, as in the baseline): 95.6%, against a baseline of 95.2%.
• scripts/check-file-length.sh at the repository root: no findings.
• Red before each fix: the new wait-timeout/marker-id test, the two busy-wait log tests and the new failure-entry test failed. The no-status fallback test passes before the change too (the old code already fell back to the wait's value and the typed id); it guards the fallback against the mutation below.
• Mutations, applied after committing and reverted afterwards:

Mutation	Result
TimeoutSeconds taken from the marker window again	Caught by TestPausePointStateErrorDetailsCarryTheWaitTimeoutAndTheMarkerId
Id always the typed id	Caught by the same test
MarkerTimeoutSeconds written even when 0	Caught by TestPausePointStateErrorDetailsFallBackToTheTypedIdWithoutAStatus
Resent correlation ids not recorded	Caught by TestRunHotReloadSendsAgainWhileACancelledExecuteDynamicCodeHoldsTheEditor
running_tool_name written for non-busy errors	Caught by TestLogPlainToolRequestFailedNamesTheRunningToolOnlyForABusyAnswer

• Checking against a live Editor was skipped: no Editor had this checkout open.

This pull request targets an integration branch, so build-and-test does not run on it; the checks above were run locally.

Not changed

• The ids in NextActions stay as typed, since they are commands to paste back.
• RemainingMilliseconds still measures the marker's window.
• cli/common and the Editor side.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: f27c5736-9bd9-4771-ab54-ef8145d9a485

📥 Commits

Reviewing files that changed from the base of the PR and between 3f1b517 and ff1e1e6.

📒 Files selected for processing (8)

• cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go
• cli/project-runner/internal/projectrunner/hot_reload_busy_wait_test.go
• cli/project-runner/internal/projectrunner/pause_point_errors.go
• cli/project-runner/internal/projectrunner/pause_point_errors_test.go
• cli/project-runner/internal/projectrunner/pause_point_wait_test.go
• cli/project-runner/internal/projectrunner/plain_tool_log.go
• cli/project-runner/internal/projectrunner/plain_tool_log_test.go
• docs/vibe-logs.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The changes add running-tool names and resend correlation IDs to busy-request logs. Pause-point error details now report the requested wait timeout separately from a positive marker timeout and use the normalized status ID when available.

Changes

Busy request logging

Layer / File(s)	Summary
Running tool details cli/project-runner/internal/projectrunner/plain_tool_log.go, cli/project-runner/internal/projectrunner/plain_tool_log_test.go, cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go, cli/project-runner/internal/projectrunner/hot_reload_busy_wait_test.go, docs/vibe-logs.md	Failed-request logs add running_tool_name for named server_busy errors. Hot-reload busy-error name extraction uses the shared helper. Tests check the log field, and the documentation describes it.
Hot-reload resend correlation IDs cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go, cli/project-runner/internal/projectrunner/hot_reload_busy_wait_test.go, docs/vibe-logs.md	Hot-reload waits record correlation IDs for resent requests and include them in completion logs. The field is an empty array when no requests were resent. Tests and documentation cover the field and its ordering.

Pause-point error details

Layer / File(s)	Summary
Pause-point timeout and ID details cli/project-runner/internal/projectrunner/pause_point_errors.go, cli/project-runner/internal/projectrunner/pause_point_errors_test.go, cli/project-runner/internal/projectrunner/pause_point_wait_test.go	Error details report the requested wait timeout as TimeoutSeconds and a positive marker timeout as MarkerTimeoutSeconds. The Id uses the status response ID when available and otherwise uses the requested ID. Tests cover both status cases.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to ff1e1

The diagnostic changes appear ready to merge after normal checks; no actionable issue remains in the supplied review.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to ff1e1

The changes clarify diagnostic output without adding command access or changing retry authority. The reviewed execution paths remain unchanged, and no material security risk was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The additional log metadata goes to the existing selected-project debug log directory. The writer remains gated by ULOOP_DEBUG and JSON-encodes entries before appending them; this PR does not change that storage implementation.

Trust Boundaries and Controls

• observed — The existing CLI-to-Editor request path is preserved. Editor-returned names enter diagnostic context only after busy-type filtering, while correlation IDs originate locally before each send and are retained on both success and failure.

Resilience and Maintainability Implications

• observed — Resend tracking is private to each wait invocation. Each completed resend increments the count and appends its ID before terminal classification, preserving attempt order across busy responses, failures, timeout, and cancellation. Completion logging consumes the returned record without changing retry decisions or introducing persistent recovery state.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.19% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 7 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the two main changes: pause-point timeout and marker ID details, plus busy log context and resend correlation IDs.
Description check	✅ Passed	The description directly explains the pause-point detail changes, busy-log changes, tests, verification results, and scope.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 76.19% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 7 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}