fix(runtime): retain decisions delivered before confirmation waits by aheritier · Pull Request #4589 · docker/docker-agent · GitHub
Skip to content

fix(runtime): retain decisions delivered before confirmation waits - #4589

Merged
aheritier merged 1 commit into
mainfrom
fix/resume-handshake-4557
Oct 15, 2026
Merged

fix(runtime): retain decisions delivered before confirmation waits#4589
aheritier merged 1 commit into
mainfrom
fix/resume-handshake-4557

Conversation

@aheritier

@aheritier aheritier commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Fixes #4557

Cause

Recovered both complete CI job logs, not just timeout cleanup:

  • Original: Go's 10-minute test timeout, TestACPRuntimeConfig_DeclaredBudgetsRemainOmittedAcrossTurns stuck 9m42s.
  • Recurrence: same test/stack, stuck 9m40s.

askUser is waiting for a resume decision while ACP has already handled the permission response and returned to its event loop. The runtime's nonblocking send on an unbuffered resume channel loses responses that arrive after prompt emission but before the receiver parks. The race-test steps ran 12m41s / 11m39s, not the 45-minute workflow budget.

Change

  • Queue one decision so an immediate prompt response is retained.
  • Discard previously queued decisions immediately before each actual prompt, under existing tool-confirmation serialization, including nested action gates and interactive iteration limits.
  • Non-interactive/already-cancelled iteration-limit paths do not drain another session's answer.
  • Regress immediate responses, stale approvals, nested late duplicates, prompt-by-prompt arming, and foreground/background overlap. Existing max-iteration tests now answer from prompt emission; all behavioral assertions remain.

No ACP source/assertion changes, timeout increases, retries, workflow or dependency changes. Requests remain unkeyed: simultaneous interactive sessions and arbitrarily late responses arriving after a new prompt is visible are a pre-existing protocol limitation, not a session-keyed approval redesign in this patch.

Validation

Local Go 1.27.2 / Ubuntu 26.04 / GCC 15.2; Task 3.53.1 and golangci-lint 2.14.0 match repository pins. CI Ubuntu 24.04.5 differs.

  • task --force build, task test, task --force lint: pass, zero lint issues.
  • CGO_ENABLED=1 task test-race: full repository pass on final tree.
  • CGO_ENABLED=1 go test -race -shuffle=on -count=20 ./pkg/runtime/... ./pkg/acp: pass (before final noninteractive-limit guard).
  • Changed resume/iteration/dispatcher tests: shuffled race 50 repetitions pass after guard correction.
  • Unchanged ACP declared-budget test: shuffled race 500 repetitions pass.
  • Same naturally scheduled CPU stress, 24 busy processes + 8 concurrent ACP binaries, GOMAXPROCS=2, -test.count=150 -test.timeout=90s: planner's baseline 2/8 processes hang with CI-identical stacks; fixed 0/8, all 1,200 iterations complete. This is stress evidence, not a CI failure-rate estimate.
  • Negative controls fail when buffering, stale-drain wiring, nested propagation or the noninteractive-limit guard is removed; restored afterwards.

An unchanged filesystem-sensitive TestFileCache_dedupSkipsRedundantWrite failed once in normal tests and once in the final full-race attempt; subsequent complete reruns passed. Separate unchanged-cache control reproduces 82/100 failures locally. No cache assertions were modified. Initial module-cache permission/tooling setup failures and test assertion lint corrections are retained in the evidence report.

Independent review found the overlapping-background iteration-limit drain; corrected and regression-tested before fresh final review. PR opened as draft for CI; no skipped race CI is counted as passed.

Fresh final independent reviewer: approve, no unresolved findings; reviewed head 1859c940367648f6323e3f09a8d567488ae96276.

@aheritier
fix(runtime): retain decisions delivered before confirmation waits
1859c94 
Queue the response raced against prompt emission, and discard decisions queued before each new tool, nested action or interactive iteration prompt. Keep non-interactive and already-cancelled limit paths from consuming another session's answer.

Signed-off-by: Arnaud Héritier <aheritier@users.noreply.github.com>
@aheritier
aheritier marked this pull request as ready for review October 13, 2026 18:53
@aheritier
aheritier requested a review from a team as a code owner October 13, 2026 18:53
Copilot AI balanced review requested due to automatic review settings October 13, 2026 18:53
Copilot AI reviewed Oct 13, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The buffered handshake and stale-decision draining are consistently wired and comprehensively regression-tested.

0 open findings

What changed in this PR

Fixes lost confirmation decisions by buffering one resume response and clearing stale decisions before new prompts.

Changes:

  • Buffers immediate resume decisions.
  • Arms tool, nested, and iteration-limit prompts against stale responses.
  • Adds regression coverage for timing and overlap scenarios.
File Description
pkg/​runtime/​runtime.go Buffers decisions and adds stale-response draining.
pkg/​runtime/​tool_dispatch.go Wires prompt preparation into dispatch.
pkg/​runtime/​loop_steps.go Arms interactive iteration-limit prompts.
pkg/​runtime/​toolexec/​dispatcher.go Prepares each tool confirmation prompt.
pkg/​runtime/​toolexec/​confirm.go Propagates preparation to nested gates.
pkg/​runtime/​resume_handshake_test.go Covers immediate, stale, nested, and overlap cases.
pkg/​runtime/​toolexec/​dispatcher_test.go Verifies per-prompt preparation.
pkg/​runtime/​loop_steps_test.go Responds after iteration prompt emission.
pkg/​runtime/​on_user_input_test.go Updates hook tests for prompt-time responses.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@aheritier aheritier added area/runtime Runtime engine, agent loop execution, tool dispatch, loop detection kind/fix PR fixes a bug (maps to fix:). Use on PRs only. labels Oct 13, 2026
@aheritier
aheritier enabled auto-merge October 13, 2026 20:42
aheritier
aheritier commented Oct 14, 2026
View reviewed changes

@aheritier aheritier left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all nine changed files and the confirmation, nested-action, iteration-limit, and ACP call paths. The bounded buffer and prompt-time draining address the observed lost-response window without broadening approval policy. The regression tests cover immediate responses, stale decisions, nested confirmations, and iteration-limit paths that do not wait. I found no actionable correctness, security, performance, or simplification findings warranting inline comments.

One validation improvement: please provide race-detector results for head 1859c94, ideally covering the affected packages and repeated handshake regressions. Required CI passed, but test-race was skipped because it only runs on main; the local race/stress results reported in the PR description were not independently verified.

@aheritier

aheritier commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Addressing review 5515463573: new race-detector executions on October 14, including a separate stateless verifier executing its own commands, all against exact head 1859c940367648f6323e3f09a8d567488ae96276. Fetched the PR head and verified the SHA and clean working tree before/after; no source changes or new commit.

Environment: Ubuntu 26.04 LTS, linux/amd64, Linux 6.12.103, Go 1.27.2, GCC 15.2.0, Task 3.53.1, 16 available CPUs. All commands explicitly set CGO_ENABLED=1. Targeted commands used set -a; source .env.test; set +a; task test-race loads the same file. All eight configured API-key/gateway variables were empty. No GORACE/GOFLAGS/GOMAXPROCS overrides. These are local results, not a hosted-CI rerun: Ubuntu differs from CI's 24.04.5, and the independent verifier shared this sandbox/toolchain while running independently authored commands.

New results

Execution Exact command (after environment above) Repetitions Exit Wall time
Author task test-race → go test -race -shuffle=on ./... Full repository once 0 191s
Author go test -race -shuffle=on -count=200 -timeout=180s -run 'TestResume|TestEnforceMaxIterations|OnUserInput|TestDispatcher_PrepareConfirmationPerPrompt' ./pkg/runtime ./pkg/runtime/toolexec 200 per selected test 0 66s
Author go test -race -shuffle=on -count=500 -timeout=180s -run '^TestACPRuntimeConfig_DeclaredBudgetsRemainOmittedAcrossTurns$' ./pkg/acp 500 0 111s
Independent verifier go test -race -shuffle=on -count=1 -timeout=180s ./pkg/runtime/... ./pkg/acp 1 0 70.829s
Independent verifier go test -race -shuffle=on -count=100 -timeout=180s -run 'TestResume|TestEnforceMaxIterations|OnUserInput|TestDispatcher_PrepareConfirmationPerPrompt' ./pkg/runtime ./pkg/runtime/toolexec 100 per selected test 0 35.221s

The handshake regex selects 20 runtime + 1 dispatcher top-level tests, plus subtests: 4,200 selected top-level executions in the author run and 2,100 in the verifier run. Counts force new execution; no (cached) result output. All five test invocations passed, with no data-race reports, assertion failures, or timeouts observed. Finite passes do not guarantee absence of future flakes.

Independent verifier's raw test output

Affected packages, count 1:

ok  github.com/docker/docker-agent/pkg/runtime             12.468s
ok  github.com/docker/docker-agent/pkg/runtime/compactor    1.035s
?   github.com/docker/docker-agent/pkg/runtime/jscommands   [no test files]
ok  github.com/docker/docker-agent/pkg/runtime/toolexec     2.141s
ok  github.com/docker/docker-agent/pkg/runtime/wire         1.611s
ok  github.com/docker/docker-agent/pkg/acp                 27.525s

Selected regressions, count 100:

ok  github.com/docker/docker-agent/pkg/runtime          29.843s
ok  github.com/docker/docker-agent/pkg/runtime/toolexec   5.320s

Raw evidence and qualifications

  • Combined new raw logs, commands, timestamps, exit statuses, environment, source provenance and checksums, artifact 7f551ac8-5f1b-44cd-81b5-2e7d87f6bd62.
  • Independent raw-log/environment archive, artifact 6727870d-643b-4c9d-8d08-d7e64a40b073; structured verification including raw logs, artifact 7b9a0096-e5e7-40a8-a9bd-b0aa77c77afe.
  • Readable new validation report, artifact 29172af1-a8bc-438f-a1c2-fbb28e813912. Artifact links use platform access controls; independent test stdout is reproduced above.
  • Fresh sandbox initially lacked GCC/Task; installed compiler/libc and repository-pinned Task. Compile-cache warmup go test -race -run '^$' ./pkg/runtime/... ./pkg/acp exited 0 but ran no tests, so is not counted as coverage. Independent verifier's initial timing harness exited 127 before running Go because /usr/bin/time was absent; retained that failure and reran with Bash built-in timing. It is not counted as a passing test invocation. No test-run timeout occurred in this follow-up.
  • Previous October 13 logs (including the unrelated cache-mtime failure and a shell-capped review attempt) remain separate in artifact 1f542133-565d-4855-96b3-94a3e582dc72; they are not relabeled as new passes. No old before/after CPU-stress experiment was rerun here.
  • GitHub CI test-race remains SKIPPED, not passed. This independent verification is a local validation result, not a GitHub approving review. No new build/lint execution is claimed for this validation-only follow-up.

@aheritier
aheritier added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit aa8fc5d Oct 15, 2026
17 checks passed
@aheritier
aheritier deleted the fix/resume-handshake-4557 branch October 15, 2026 06:04
@BrewTestBot BrewTestBot mentioned this pull request Oct 19, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@trungutt trungutt trungutt approved these changes

Assignees

No one assigned

Labels

area/runtime Runtime engine, agent loop execution, tool dispatch, loop detection kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[CI failure] Main test-race job failed

3 participants

@aheritier @trungutt