Skip to content

Add background cmd output check in stages 5 and 6#270

Open
UdeshyaDhungana wants to merge 2 commits intobg-jobs-3from
add-bg-output-check
Open

Add background cmd output check in stages 5 and 6#270
UdeshyaDhungana wants to merge 2 commits intobg-jobs-3from
add-bg-output-check

Conversation

@UdeshyaDhungana
Copy link
Contributor

@UdeshyaDhungana UdeshyaDhungana commented Mar 2, 2026

Note

Medium Risk
Changes the background-jobs test harness and expectations, which could introduce false positives/negatives across stages if prompt/output interleaving differs between shells.

Overview
Tightens stages bg5/bg6 to verify that background grep jobs actually emit the matched line, switching from grep -q to grep and adding output assertions after writing to the FIFO.

Introduces BackgroundCommandOutputOnlyTestCase to assert background command output without requiring a prompt, and updates JobsBuiltinResponseTestCase with SkipCurrentPromptAssertion to handle cases where background output consumes the prompt line.

Updates the Python fixture shell to stop discarding background process output, refreshes fixtures accordingly, and adds an indirect dependency on al.essio.dev/pkg/shellescape for quoting commands in success messages.

Written by Cursor Bugbot for commit 427041c. This will update automatically on new commits. Configure here.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Inconsistent sleep placement creates potential race condition
    • Moved time.Sleep from before the BackgroundCommandOutputOnlyTestCase assertion to after it in the second write flow, matching the first flow's pattern and ensuring grep has time to exit before the jobs command runs.

Create PR

Or push these changes by commenting:

@cursor push 9b66b3949c
Preview (9b66b3949c)
diff --git a/internal/stage_bg6.go b/internal/stage_bg6.go
--- a/internal/stage_bg6.go
+++ b/internal/stage_bg6.go
@@ -107,8 +107,6 @@
 		return err
 	}
 
-	time.Sleep(time.Millisecond)
-
 	// Assert the background command's output
 	err = test_cases.BackgroundCommandOutputOnlyTestCase{
 		ExpectedOutputLines: []string{grepPattern2},
@@ -119,6 +117,8 @@
 		return err
 	}
 
+	time.Sleep(time.Millisecond)
+
 	// Call jobs for the second time
 	jobsBuiltinTestCase2 := test_cases.JobsBuiltinResponseTestCase{
 		ExpectedOutputEntries: []test_cases.JobsBuiltinOutputEntry{

@UdeshyaDhungana UdeshyaDhungana changed the title Add background command output check in stages 5 and 6 Add background cmd output check in stages 5 and 6 Mar 2, 2026
ExpectedOutputEntries []JobsBuiltinOutputEntry
SuccessMessage string
// SkipCurrentPromptAssertion should be set to true if the prompt symbol is not expected in the 'jobs' command reflection
SkipCurrentPromptAssertion bool
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
SkipCurrentPromptAssertion bool
ShouldSkipCurrentPromptAssertion bool

(Stick to convention for boolean flags)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants