Skip to content

test(log): exercise foreign fd-capture sentinel in reader path - #141

Open
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/pull-request-comment-fixes-c0ca
Open

test(log): exercise foreign fd-capture sentinel in reader path#141
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/pull-request-comment-fixes-c0ca

Conversation

@cursor

@cursor cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

Follow-up to #140 (merged before the CodeRabbit inline comment could land on the head branch).

test_child_stop_leaks_no_control_bytes only exercised the child no-op stop() path. Child stop() never writes a sentinel off the owner pid, so _reader_loop’s foreign PID-tagged sentinel handling was untested. After waitpid(), write _stop_sentinel(pid) to fd 2 and keep the existing assertions that the marker is not logged/teed and later output still captures.

Addresses: #140 (comment)

Tested (run the relevant ones):

  • poetry run pytest tests/test_fd_capture.py::TestForkedChildCannotDisableParentCapture -rs — 3 passed
  • Code formatting: bash format.sh
Open in Web View Automation 

Note

Low Risk
Test-only change; no production fd-capture behavior is modified.

Overview
Extends test_child_stop_leaks_no_control_bytes so it actually covers _reader_loop handling of a foreign PID-tagged stop sentinel, not only the child’s no-op stop() (which never writes a sentinel off the owner pid).

After waitpid, the parent writes fc._stop_sentinel(pid) to fd 2; existing checks still require later output to be captured, with no fdcap:flush or null bytes in logged lines.

Reviewed by Cursor Bugbot for commit bce6a84. Configure here.

Child stop() is a no-op off the owner pid, so the existing fork test never
wrote a PID-tagged sentinel. Inject one after waitpid so _reader_loop's
foreign-sentinel drop path is covered.

Co-authored-by: Andrew Aikawa <asai@berkeley.edu>
@cursor
cursor Bot temporarily deployed to integration August 6, 2026 18:05 Inactive
@cursor
cursor Bot deployed to integration August 6, 2026 18:05 Active
@asaiacai
asaiacai marked this pull request as ready for review August 6, 2026 23:08
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.

1 participant