Repository navigation
Conversation
hugodutka
changed the title
Author
|
Superseded by the requested approach: synchronize ACP SDK logger access and explicitly route ACP diagnostics through Coder logging. The replacement will be linked from coder/coder#31405. Generated by Coder Agents on behalf of @hugodutka. |
hugodutka
added a commit
to coder/coder
that referenced
this pull request
Oct 29, 2026
Route ACP SDK diagnostics through the workspace agent's Coder logger instead of the process-wide standard slog logger, which Trivy replaces with an unsafe buffering handler. Install the logger before starting the harness so even immediate output and disconnects use the configured logger. Bump the ACP SDK to commit `0da7315c23b5`, which merged the synchronized `SetLogger` fix into SDK main in coder/acp-go-sdk#59. Preserve diagnostic levels and structured fields, identify the harness, and omit raw protocol frames that can contain conversation content or workspace secrets. The previous Trivy dependency update is removed; coder/trivy#87 is closed as superseded. Fixes https://linear.app/codercom/issue/ENG-3489/flake-testagent-acpreadiness-data-race-in-trivy-slog-deferredhandler Generated by Coder Agents on behalf of @hugodutka.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trivy's process-wide default slog handler buffers records without synchronization. Concurrent ACP connection shutdowns in Coder race while appending to that buffer.
Protect the shared buffer across derived handlers, clone retained records, and forward subsequent records to the initialized handler after flushing. Add regression coverage for concurrent writers and concurrent flushing.
Consumed by coder/coder#31405.
Refs https://linear.app/codercom/issue/ENG-3489/flake-testagent-acpreadiness-data-race-in-trivy-slog-deferredhandler
Implementation and validation evidence
The mutex belongs to shared state rather than the handler itself so handlers created with
WithAttrssynchronize access to the same buffer. Flushing detaches the buffer under the lock and invokes the target handler outside the lock. Existing loggers forward to that target instead of appending to an abandoned buffer or dereferencing nil state.go test -race -shuffle=on -count=100 ./pkg/logpassed. Removing synchronization made both new tests fail with data races and lost records.golangci-lint run ./pkg/log/...passed with no issues.The matching Coder CI failure is https://github.com/coder/coder/actions/runs/40600292300.
Generated by Coder Agents on behalf of @hugodutka.