fix(backgroundjobs): resolve status race condition and subprocess pipe hang

[Piyush0049]

Summary

This pull request resolves a critical status race condition in background jobs and eliminates process hangs caused by inherited I/O pipes from background subprocesses. It also ensures cross-platform test reliability on Windows and silences platform-specific linter false-positives.

Root Cause & Changes

1. Atomic Status Transitions (CompareAndSwap)

• Issue: Previously, monitorJob used blind status writes (job.status.Store(...)). If StopBackgroundJob or ToolSet.Stop terminated a job at the exact moment a process was exiting naturally, monitorJob could overwrite statusStopped with statusCompleted or statusFailed. This caused erroneous recall steering messages to be sent back to the AI model for manually aborted jobs.
• Fix: Replaced blind store assignments in monitorJob with atomic Compare-And-Swap (CAS) transitions (job.status.CompareAndSwap(statusRunning, newStatus)). Once a job transitions out of statusRunning, subsequent background monitoring completions cannot overwrite the terminated state.

2. Piped Subprocess Cleanup (cmd.WaitDelay)

• Issue: When background commands spawned child or grandchild processes that inherited open stdout/stderr file descriptors (such as backgrounded scripts or daemons), Go's cmd.Wait() would block indefinitely waiting for an EOF on the pipes even after the primary process terminated.
• Fix: Configured cmd.WaitDelay = 1 * time.Second on all executed background jobs to automatically force-close orphaned I/O pipes if child processes hold them open after the primary process exits.

3. Cross-Platform Compatibility & Linting

• Test Reliability: Updated TestResolveWorkDir in backgroundjobs_test.go to use filepath.FromSlash and conditional OS root prefixes (C:\... on Windows) to prevent false-positive path resolution failures on Windows.
• Linting Harmony: Updated exec_windows.go to use exec.CommandContext and wrapped secondary fallback errors with %w. Added ,nolintlint to cross-platform silencing directives (//nolint:gosec in snapshot.go and //nolint:bodyclose in transport_test.go) so that running golangci-lint on Windows does not flag them as unused.

[aheritier]

Hi @Piyush0049

Please provide more details about the issue you are trying to solve. Is it something which happens often? What are the symptoms?

Right now the PR doesn't provide enough context to understand if you are fixing a bug or if these are just code reviews by an LLM which could easily hallucinate bugs.

Thanks

[Piyush0049]

Hi @aheritier,

• Official Go Docs: pkg.go.dev/os/exec#Cmd.WaitDelay

"If WaitDelay is zero (the default), Wait waits indefinitely for I/O to complete."

While reproducing this, I noticed two separate issues:

1. cmd.Wait() can hang

If a child/background process keeps the inherited stdout/stderr pipes open, cmd.Wait() can block indefinitely because WaitDelay is 0 by default.

Setting:

cmd.WaitDelay = time.Second

prevents Wait() from hanging forever in this scenario.

2. Status race condition

There also seems to be a race between StopBackgroundJob() and monitorJob().

If the user stops a job while monitorJob() is finishing, the final Store(statusFailed) can overwrite statusStopped.

Using:

job.status.CompareAndSwap(statusRunning, newStatus)

ensures monitorJob() only updates the status if the job is still running, so a manually stopped job remains stopped.

[docker-agent]

Assessment: 🟢 APPROVE

The changes are well-structured and correctly address the stated goals:

• CAS in monitorJob: The mutex acquisition and CompareAndSwap ordering is correct. The mutex is held at the point of the early unlock on CAS failure — no double-unlock or missed unlock. The CAS prevents monitorJob from overwriting a statusStopped state set by StopBackgroundJob.
• cmd.WaitDelay: Appropriately handles orphaned child processes holding open pipes. ErrWaitDelay (non-ExitError) is correctly routed to statusFailed with exit code -1.
• Multiple %w in fmt.Errorf: Supported since Go 1.20; this project targets Go 1.26.5 — no issue.
• exec.CommandContext: The gosec G204 nolint removal is safe because G204 is globally excluded in .golangci.yml.
• nolintlint additions: Correctly suppress the nolintlint false-positives on Windows builds where the guarded linters don't run.