fix(core): guard bash streaming test callback against concurrent append

[ezynda3]

Description

executeBashStreaming drains stdout and stderr in two separate goroutines, so the ToolOutputCallback it invokes is called concurrently. The production code guards its own stdoutChunks / stderrChunks with a mutex, but the test helper added in #110 appended to a shared slice with no synchronisation:

var chunks []string
cb := func(_, _, chunk string, _ bool) {
    chunks = append(chunks, chunk)   // called from both stream goroutines
}

Under -race this is a genuine data race. It surfaces only under load — the full go test -race ./..., or -cpu=1,2,4 — which is why CI on #110 went green and the package still passes in isolation. I found it while validating a merge of master into another branch.

This is a test-only fix. executeBashStreaming itself was already correct.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactor / chore

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation — not applicable, test-only
• My changes generate no new warnings (go vet, gofmt, golangci-lint all clean)
• I have added tests — the change is to a test; no new test added
• New and existing unit tests pass locally with my changes (go test -race ./...)

Additional Information

Verification

Reproduced on master before the fix:

$ go test -race -count=1 -cpu=1,2,4 ./internal/core/ -run TestBashStreaming
WARNING: DATA RACE
Read at 0x00c00012e2a0 by goroutine 16:
  internal/core.runStreaming.func1.1()
      internal/core/bash_streaming_test.go:33
  internal/core.executeBashStreaming.func1()
      internal/core/bash.go:405

After the fix, three consecutive -cpu=1,2,4 rounds are clean, plus a full go test -race -count=1 ./....

The fix

The callback locks around the append, and the final slice is published as a snapshot taken under the same mutex. executeBashStreaming has joined both stream goroutines by the time it returns, so no callback can still be in flight — the final lock is for correct publication rather than mutual exclusion, and the comment says so.

Note

Introduced by me in #110. CI did not catch it because the race needs concurrent load to manifest, and the test job runs the suite in a single pass where scheduling happened to serialise the two goroutines.

[mark-iii-labs-huly]

Connected to Huly®: KIT-112

[coderabbitai]

Warning

Review limit reached

Next included review available in 10 minutes.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5c4910fd-739d-409f-a87c-a967e560a85d

📥 Commits

Reviewing files that changed from the base of the PR and between a279dcc and d44b131.

📒 Files selected for processing (1)

• internal/core/bash_streaming_test.go

---

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.}