From eaf1f6a65e85c29dd388047c6ebc9ad345247dbe Mon Sep 17 00:00:00 2001 From: Aleksey Safonov <55020240+safal207@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:26:14 +0300 Subject: [PATCH 1/9] docs: add Boris and Greg upstream contributor plan --- contrib/upstream-contributor-kits/README.md | 76 +++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 contrib/upstream-contributor-kits/README.md diff --git a/contrib/upstream-contributor-kits/README.md b/contrib/upstream-contributor-kits/README.md new file mode 100644 index 00000000..06cdddc4 --- /dev/null +++ b/contrib/upstream-contributor-kits/README.md @@ -0,0 +1,76 @@ +# Upstream contributor kits: Boris + Greg + +This directory prepares two small, reviewable upstream contributions. + +## Boris Cherny / `anthropics/claude-code-action#1522` + +Target revision: + +```text +anthropics/claude-code-action +b76a0776ae74036e77cd11018083743453d7ad35 +``` + +Problem: + +```text +pull_request_review webhook + -> review.submitted_at becomes triggerTime + -> filterReviewsToTriggerTime rejects submittedAt >= triggerTime + -> the triggering review is removed + -> its inline comments disappear from prompt context +``` + +Candidate contract: + +```text +retain only the review whose databaseId equals the webhook review ID +keep strict same-time rejection for every other review +exclude any review edited strictly after the trigger +retain inline comments +avoid duplicating the review body already supplied by the webhook +``` + +The generated patch is tested with the repository's Bun unit tests, typecheck, +and format check. + +## Greg Brockman / `gdb/tee-output#3` + +Target revision: + +```text +gdb/tee-output +c41f8ff383200320b746e953e92709ae1b505a71 +``` + +Problem boundary: + +```text +write end closes + -> SIGINT is sent immediately to parent-lifetime + -> underlying tee may be interrupted before startup/tail drain completes +``` + +Candidate contract: + +```text +flush Python streams +close writer and deliver EOF +wait for natural reader completion +use SIGINT only as a bounded fallback +``` + +The generated patch adds a PTY regression test for: + +```text +print -> traceback -> immediate Tee.close() +``` + +and runs it on Linux and macOS. + +## Authority boundary + +These are contributor-ready candidates, not claims that upstream has accepted a +root cause or fix. No upstream branch, issue state, label, review, or merge is +modified by this repository. An external PR requires a GitHub fork owned by the +user. \ No newline at end of file From b218548ebf4a912e5f117b6ec6567412f4747ec5 Mon Sep 17 00:00:00 2001 From: Aleksey Safonov <55020240+safal207@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:26:38 +0300 Subject: [PATCH 2/9] test: add fail-closed Boris patch generator --- .../apply_boris_1522.py | 360 ++++++++++++++++++ 1 file changed, 360 insertions(+) create mode 100644 contrib/upstream-contributor-kits/apply_boris_1522.py diff --git a/contrib/upstream-contributor-kits/apply_boris_1522.py b/contrib/upstream-contributor-kits/apply_boris_1522.py new file mode 100644 index 00000000..a21aa4c9 --- /dev/null +++ b/contrib/upstream-contributor-kits/apply_boris_1522.py @@ -0,0 +1,360 @@ +#!/usr/bin/env python3 +# Apply the bounded fix for anthropics/claude-code-action#1522. +# Every replacement must match once against the pinned upstream revision. + +from __future__ import annotations + +import argparse +import subprocess +from pathlib import Path + + +def replace_once(path: Path, old: str, new: str) -> None: + text = path.read_text(encoding="utf-8") + count = text.count(old) + if count != 1: + raise RuntimeError(f"{path}: expected one source match, found {count}") + path.write_text(text.replace(old, new, 1), encoding="utf-8") + + +def main() -> None: + parser = argparse.ArgumentParser() + parser.add_argument("repo", type=Path) + parser.add_argument("--patch-out", type=Path, required=True) + args = parser.parse_args() + repo = args.repo.resolve() + + fetcher = repo / "src/github/data/fetcher.ts" + tag = repo / "src/modes/tag/index.ts" + tests = repo / "test/data-fetcher.test.ts" + + replace_once( + fetcher, + ''' return undefined; +} + +/** + * Extracts the original title from the GitHub webhook payload. +''', + ''' return undefined; +} + +/** + * Returns the database ID of the review that triggered a + * pull_request_review event. + * + * The ID lets the trigger-time filter retain exactly the webhook entity while + * continuing to reject unrelated reviews submitted at the same timestamp. + */ +export function extractTriggerReviewDatabaseId( + context: ParsedGitHubContext, +): string | undefined { + return isPullRequestReviewEvent(context) + ? context.payload.review.id.toString() + : undefined; +} + +/** + * Extracts the original title from the GitHub webhook payload. +''', + ) + + replace_once( + fetcher, + '''export function filterReviewsToTriggerTime< + T extends { submittedAt: string; updatedAt?: string; lastEditedAt?: string }, +>(reviews: T[], triggerTime: string | undefined): T[] { + if (!triggerTime) return reviews; + + const triggerTimestamp = new Date(triggerTime).getTime(); + + return reviews.filter((review) => { + // Review must have been submitted before trigger (not at or after) + const submittedTimestamp = new Date(review.submittedAt).getTime(); + if (submittedTimestamp >= triggerTimestamp) { + return false; + } + + // If review has been edited, the most recent edit must have occurred before trigger + const lastEditTime = review.lastEditedAt || review.updatedAt; + if (lastEditTime) { + const lastEditTimestamp = new Date(lastEditTime).getTime(); + if (lastEditTimestamp >= triggerTimestamp) { + return false; + } + } + + return true; + }); +} +''', + '''export function filterReviewsToTriggerTime< + T extends { + databaseId?: string; + submittedAt: string; + updatedAt?: string; + lastEditedAt?: string; + }, +>( + reviews: T[], + triggerTime: string | undefined, + triggerReviewDatabaseId?: string, +): T[] { + if (!triggerTime) return reviews; + + const triggerTimestamp = new Date(triggerTime).getTime(); + + return reviews.filter((review) => { + const isTriggerReview = + triggerReviewDatabaseId !== undefined && + review.databaseId === triggerReviewDatabaseId; + + // Reviews after the trigger are unsafe. Reviews exactly at the trigger are + // only safe when their database ID matches the webhook entity. + const submittedTimestamp = new Date(review.submittedAt).getTime(); + if ( + submittedTimestamp > triggerTimestamp || + (submittedTimestamp === triggerTimestamp && !isTriggerReview) + ) { + return false; + } + + // Preserve the exact triggering review if GitHub reports its update time as + // the submission time. Any edit strictly after the trigger remains unsafe. + const lastEditTime = review.lastEditedAt || review.updatedAt; + if (lastEditTime) { + const lastEditTimestamp = new Date(lastEditTime).getTime(); + if ( + lastEditTimestamp > triggerTimestamp || + (lastEditTimestamp === triggerTimestamp && !isTriggerReview) + ) { + return false; + } + } + + return true; + }); +} +''', + ) + + replace_once( + fetcher, + ''' triggerUsername?: string; + triggerTime?: string; + originalTitle?: string; +''', + ''' triggerUsername?: string; + triggerTime?: string; + triggerReviewDatabaseId?: string; + originalTitle?: string; +''', + ) + + replace_once( + fetcher, + ''' triggerUsername, + triggerTime, + originalTitle, +''', + ''' triggerUsername, + triggerTime, + triggerReviewDatabaseId, + originalTitle, +''', + ) + + replace_once( + fetcher, + ''' reviewData.nodes = filterCommentsByActor( + filterReviewsToTriggerTime(reviewData.nodes, triggerTime), + includeCommentsByActor, + excludeCommentsByActor, + ); + + // Apply the same trigger-time + actor filtering to inline review comments. +''', + ''' reviewData.nodes = filterCommentsByActor( + filterReviewsToTriggerTime( + reviewData.nodes, + triggerTime, + triggerReviewDatabaseId, + ), + includeCommentsByActor, + excludeCommentsByActor, + ); + + // The webhook payload already provides the triggering review body through + // trigger_comment. Keep the review node for its inline comments and state, + // but avoid duplicating the body in . + reviewData.nodes = reviewData.nodes.map((review) => + review.databaseId === triggerReviewDatabaseId + ? { ...review, body: "" } + : review, + ); + + // Apply the same trigger-time + actor filtering to inline review comments. +''', + ) + + replace_once( + tag, + ''' extractTriggerTimestamp, + extractOriginalTitle, + extractOriginalBody, +''', + ''' extractTriggerTimestamp, + extractTriggerReviewDatabaseId, + extractOriginalTitle, + extractOriginalBody, +''', + ) + replace_once( + tag, + ''' const triggerTime = extractTriggerTimestamp(context); + const originalTitle = extractOriginalTitle(context); +''', + ''' const triggerTime = extractTriggerTimestamp(context); + const triggerReviewDatabaseId = extractTriggerReviewDatabaseId(context); + const originalTitle = extractOriginalTitle(context); +''', + ) + replace_once( + tag, + ''' triggerUsername: context.actor, + triggerTime, + originalTitle, +''', + ''' triggerUsername: context.actor, + triggerTime, + triggerReviewDatabaseId, + originalTitle, +''', + ) + + replace_once( + tests, + ''' extractTriggerTimestamp, + extractOriginalTitle, +''', + ''' extractTriggerTimestamp, + extractTriggerReviewDatabaseId, + extractOriginalTitle, +''', + ) + replace_once( + tests, + '''describe("extractOriginalTitle", () => { +''', + '''describe("extractTriggerReviewDatabaseId", () => { + it("should extract the triggering review database ID", () => { + expect(extractTriggerReviewDatabaseId(mockPullRequestReviewContext)).toBe( + "11122233", + ); + }); + + it("should return undefined for non-review events", () => { + expect(extractTriggerReviewDatabaseId(mockIssueCommentContext)).toBeUndefined(); + expect( + extractTriggerReviewDatabaseId(mockPullRequestReviewCommentContext), + ).toBeUndefined(); + }); +}); + +describe("extractOriginalTitle", () => { +''', + ) + replace_once( + tests, + ''' const createMockReview = ( + submittedAt: string, + updatedAt?: string, + lastEditedAt?: string, + ): GitHubReview => ({ + id: String(Math.random()), + databaseId: String(Math.random()), +''', + ''' const createMockReview = ( + submittedAt: string, + updatedAt?: string, + lastEditedAt?: string, + databaseId: string = String(Math.random()), + ): GitHubReview => ({ + id: String(Math.random()), + databaseId, +''', + ) + replace_once( + tests, + ''' it("should handle exact timestamp match", () => { + const review = createMockReview("2024-01-15T12:00:00Z"); + const filtered = filterReviewsToTriggerTime([review], triggerTime); + // Reviews submitted exactly at trigger time should be excluded for security + expect(filtered.length).toBe(0); + }); +''', + ''' it("should exclude an unrelated review at the exact trigger timestamp", () => { + const review = createMockReview("2024-01-15T12:00:00Z"); + const filtered = filterReviewsToTriggerTime( + [review], + triggerTime, + "trigger-review-id", + ); + expect(filtered.length).toBe(0); + }); + + it("should include the exact triggering review at the trigger timestamp", () => { + const review = createMockReview( + "2024-01-15T12:00:00Z", + undefined, + undefined, + "trigger-review-id", + ); + const filtered = filterReviewsToTriggerTime( + [review], + triggerTime, + "trigger-review-id", + ); + expect(filtered).toEqual([review]); + }); + + it("should include the triggering review when updatedAt equals submission time", () => { + const review = createMockReview( + "2024-01-15T12:00:00Z", + "2024-01-15T12:00:00Z", + undefined, + "trigger-review-id", + ); + const filtered = filterReviewsToTriggerTime( + [review], + triggerTime, + "trigger-review-id", + ); + expect(filtered).toEqual([review]); + }); + + it("should still exclude the triggering review if it was edited after the trigger", () => { + const review = createMockReview( + "2024-01-15T12:00:00Z", + "2024-01-15T12:00:01Z", + undefined, + "trigger-review-id", + ); + const filtered = filterReviewsToTriggerTime( + [review], + triggerTime, + "trigger-review-id", + ); + expect(filtered.length).toBe(0); + }); +''', + ) + + subprocess.run(["git", "-C", str(repo), "diff", "--check"], check=True) + args.patch_out.parent.mkdir(parents=True, exist_ok=True) + patch = subprocess.check_output(["git", "-C", str(repo), "diff", "--binary"]) + args.patch_out.write_bytes(patch) + + +if __name__ == "__main__": + main() From a6e5384cbc72286dbaeb12a3b7b7a9bb90c10f27 Mon Sep 17 00:00:00 2001 From: Aleksey Safonov <55020240+safal207@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:27:33 +0300 Subject: [PATCH 3/9] test: add fail-closed Greg patch generator --- .../upstream-contributor-kits/apply_greg_3.py | 268 ++++++++++++++++++ 1 file changed, 268 insertions(+) create mode 100644 contrib/upstream-contributor-kits/apply_greg_3.py diff --git a/contrib/upstream-contributor-kits/apply_greg_3.py b/contrib/upstream-contributor-kits/apply_greg_3.py new file mode 100644 index 00000000..4ed61f58 --- /dev/null +++ b/contrib/upstream-contributor-kits/apply_greg_3.py @@ -0,0 +1,268 @@ +#!/usr/bin/env python3 +# Apply the bounded candidate fix for gdb/tee-output#3. +# Every source replacement must match once against the pinned upstream revision. + +from __future__ import annotations + +import argparse +import subprocess +from pathlib import Path + + +def replace_once(path: Path, old: str, new: str) -> None: + text = path.read_text(encoding="utf-8") + count = text.count(old) + if count != 1: + raise RuntimeError(f"{path}: expected one source match, found {count}") + path.write_text(text.replace(old, new, 1), encoding="utf-8") + + +TEST_FILE = r'''import errno +import os +import pty +import subprocess +import sys +import tempfile +import textwrap +import unittest +from pathlib import Path + + +CHILD = textwrap.dedent( + r""" + import sys + import traceback + from pathlib import Path + from tee_output import Tee + + root = Path(sys.argv[1]) + stdout = [str(root / "stdout.log"), str(root / "combined.log")] + stderr = [str(root / "stderr.log"), str(root / "combined.log")] + + tee = Tee().to(stdout=stdout, stderr=stderr) + print("stdout-marker", flush=True) + try: + raise RuntimeError("stderr-marker") + except RuntimeError: + traceback.print_exc() + finally: + tee.close() + """ +) + + +def read_terminal(master): + chunks = [] + while True: + try: + chunk = os.read(master, 65536) + except OSError as exc: + if exc.errno == errno.EIO: + break + raise + if not chunk: + break + chunks.append(chunk) + return b"".join(chunks) + + +class ImmediateCloseTest(unittest.TestCase): + def test_print_traceback_immediate_close_preserves_all_outputs(self): + for round_id in range(10): + with self.subTest(round=round_id), tempfile.TemporaryDirectory() as tmp: + master, slave = pty.openpty() + try: + proc = subprocess.Popen( + [sys.executable, "-u", "-c", CHILD, tmp], + stdin=slave, + stdout=slave, + stderr=slave, + close_fds=True, + ) + finally: + os.close(slave) + + terminal = read_terminal(master) + os.close(master) + self.assertEqual( + proc.wait(timeout=10), + 0, + terminal.decode(errors="replace"), + ) + + root = Path(tmp) + self.assertIn("stdout-marker", (root / "stdout.log").read_text()) + self.assertIn("stderr-marker", (root / "stderr.log").read_text()) + combined = (root / "combined.log").read_text() + self.assertIn("stdout-marker", combined) + self.assertIn("stderr-marker", combined) + + +if __name__ == "__main__": + unittest.main() +''' + + +WORKFLOW_FILE = r'''name: test + +on: + pull_request: + push: + +permissions: + contents: read + +jobs: + test: + strategy: + fail-fast: false + matrix: + os: + - ubuntu-latest + - macos-latest + python: + - "3.11" + - "3.13" + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - run: python -m pip install --upgrade pip + - run: python -m pip install -e . + - run: python -m unittest discover -s tests -v +''' + + +def main() -> None: + parser = argparse.ArgumentParser() + parser.add_argument("repo", type=Path) + parser.add_argument("--patch-out", type=Path, required=True) + args = parser.parse_args() + repo = args.repo.resolve() + + tee = repo / "tee_output/__init__.py" + parent = repo / "bin/parent-lifetime" + + replace_once( + tee, + ''' def close(self): + self.pause() + self._drain(self.stdout_pipe_proc, self.stderr_pipe_proc) + + def _drain(self, stdout_pipe_proc, stderr_pipe_proc): + # One sharp edge is that if you've spawned a subprocess with + # the redirected stdout/stderr, the tee processes will not + # die. In that case maybe we should set a timeout, or just + # leak them? Not sure. + if stdout_pipe_proc is not None: + pipe, proc = stdout_pipe_proc + pipe.close() + try: + # TODO: replace tee with something that is guaranteed + # to flush on exit + os.kill(proc.pid, signal.SIGINT) + except ProcessLookupError: + pass + proc.wait() + if stderr_pipe_proc is not None: + pipe, proc = stderr_pipe_proc + pipe.close() + try: + os.kill(proc.pid, signal.SIGINT) + except ProcessLookupError: + pass + proc.wait() +''', + ''' def close(self): + # Flush Python-level buffers before restoring the original descriptors. + # This is separate from draining the external tee processes below. + sys.stdout.flush() + sys.stderr.flush() + self.pause() + self._drain(self.stdout_pipe_proc, self.stderr_pipe_proc) + + def _drain(self, stdout_pipe_proc, stderr_pipe_proc): + # One sharp edge is that if you've spawned a subprocess with + # the redirected stdout/stderr, the tee processes will not die from EOF. + # Preserve bounded shutdown for that case, but do not interrupt the + # normal path before tee has had a chance to drain. + self._drain_one(stdout_pipe_proc) + self._drain_one(stderr_pipe_proc) + + @staticmethod + def _drain_one(pipe_proc): + if pipe_proc is None: + return + + pipe, proc = pipe_proc + pipe.close() + + try: + # Closing the final writer delivers EOF. Let tee consume the + # buffered PTY/pipe tail and exit naturally before using signals. + proc.wait(timeout=2) + return + except subprocess.TimeoutExpired: + pass + + try: + # Fallback for inherited descriptors or a stuck reader stack. + os.kill(proc.pid, signal.SIGINT) + except ProcessLookupError: + return + proc.wait() +''', + ) + + replace_once( + parent, + ''' # Let the process die naturally + while True: + if child.poll() is not None: + return child.returncode + elif os.getppid() == 1: + break + time.sleep(poll_timeout) +''', + ''' # Let the process die naturally. Waiting on the child directly observes + # EOF-driven completion immediately; the timeout only provides a cadence + # for checking whether our own parent disappeared. + while True: + try: + child.wait(timeout=poll_timeout) + return child.returncode + except subprocess.TimeoutExpired: + if os.getppid() == 1: + break +''', + ) + + tests = repo / "tests" + tests.mkdir(exist_ok=True) + (tests / "test_immediate_close.py").write_text(TEST_FILE, encoding="utf-8") + + workflow = repo / ".github/workflows" + workflow.mkdir(parents=True, exist_ok=True) + (workflow / "test.yml").write_text(WORKFLOW_FILE, encoding="utf-8") + + subprocess.run( + [ + "python", + "-m", + "py_compile", + str(tee), + str(parent), + str(tests / "test_immediate_close.py"), + ], + check=True, + ) + subprocess.run(["git", "-C", str(repo), "diff", "--check"], check=True) + args.patch_out.parent.mkdir(parents=True, exist_ok=True) + patch = subprocess.check_output(["git", "-C", str(repo), "diff", "--binary"]) + args.patch_out.write_bytes(patch) + + +if __name__ == "__main__": + main() From f6c56bf59a14b2d2ee28751e85509ae3ec5d7d9f Mon Sep 17 00:00:00 2001 From: Aleksey Safonov <55020240+safal207@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:27:59 +0300 Subject: [PATCH 4/9] ci: validate upstream contributor kits --- .../workflows/upstream-contributor-kits.yml | 180 ++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 .github/workflows/upstream-contributor-kits.yml diff --git a/.github/workflows/upstream-contributor-kits.yml b/.github/workflows/upstream-contributor-kits.yml new file mode 100644 index 00000000..1759cde3 --- /dev/null +++ b/.github/workflows/upstream-contributor-kits.yml @@ -0,0 +1,180 @@ +name: Upstream Contributor Kits + +on: + pull_request: + paths: + - "contrib/upstream-contributor-kits/**" + - ".github/workflows/upstream-contributor-kits.yml" + push: + branches: + - "agent/upstream-contributor-kits-v0-1" + paths: + - "contrib/upstream-contributor-kits/**" + - ".github/workflows/upstream-contributor-kits.yml" + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: upstream-contributor-kits-${{ github.ref }} + cancel-in-progress: true + +jobs: + boris-claude-code-action-1522: + name: Boris #1522 exact patch + runs-on: ubuntu-latest + steps: + - name: Checkout contributor kit + uses: actions/checkout@v4 + + - name: Checkout exact Claude Code Action source + uses: actions/checkout@v4 + with: + repository: anthropics/claude-code-action + ref: b76a0776ae74036e77cd11018083743453d7ad35 + path: upstream/claude-code-action + persist-credentials: false + + - name: Set up Bun + uses: oven-sh/setup-bun@v2 + + - name: Install exact dependencies + working-directory: upstream/claude-code-action + run: bun install --frozen-lockfile + + - name: Apply fail-closed triggering-review patch + run: | + python contrib/upstream-contributor-kits/apply_boris_1522.py \ + upstream/claude-code-action \ + --patch-out artifacts/boris/claude-code-action-1522.patch + + - name: Format changed TypeScript + working-directory: upstream/claude-code-action + run: | + bunx prettier --write \ + src/github/data/fetcher.ts \ + src/modes/tag/index.ts \ + test/data-fetcher.test.ts + + - name: Run focused regression tests + working-directory: upstream/claude-code-action + run: bun test test/data-fetcher.test.ts + + - name: Run typecheck + working-directory: upstream/claude-code-action + run: bun run typecheck + + - name: Verify formatting and patch integrity + working-directory: upstream/claude-code-action + run: | + bun run format:check + git diff --check + git diff --binary > ../../artifacts/boris/claude-code-action-1522.patch + test -s ../../artifacts/boris/claude-code-action-1522.patch + + - name: Record exact Boris receipt + run: | + cat > artifacts/boris/receipt.txt <<'EOF' + target=anthropics/claude-code-action + issue=1522 + upstream_sha=b76a0776ae74036e77cd11018083743453d7ad35 + contract=retain_exact_triggering_review_by_database_id + same_timestamp_unrelated_review=excluded + post_trigger_edit=excluded + triggering_inline_comments=retained + triggering_review_body=deduplicated_from_review_context + EOF + sha256sum artifacts/boris/claude-code-action-1522.patch >> artifacts/boris/receipt.txt + + - name: Upload Boris contributor kit + uses: actions/upload-artifact@v4 + with: + name: boris-claude-code-action-1522-${{ github.run_id }}-${{ github.run_attempt }} + path: artifacts/boris + if-no-files-found: error + + greg-tee-output-3: + name: Greg #3 ${{ matrix.os }} Python ${{ matrix.python }} + strategy: + fail-fast: false + matrix: + os: + - ubuntu-latest + - macos-latest + python: + - "3.11" + - "3.13" + runs-on: ${{ matrix.os }} + steps: + - name: Checkout contributor kit + uses: actions/checkout@v4 + + - name: Checkout exact tee-output source + uses: actions/checkout@v4 + with: + repository: gdb/tee-output + ref: c41f8ff383200320b746e953e92709ae1b505a71 + path: upstream/tee-output + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + + - name: Apply EOF-before-SIGINT candidate + run: | + python contrib/upstream-contributor-kits/apply_greg_3.py \ + upstream/tee-output \ + --patch-out artifacts/greg/tee-output-3.patch + + - name: Install exact candidate + working-directory: upstream/tee-output + run: | + python -m pip install --upgrade pip + python -m pip install -e . + + - name: Run PTY immediate-close regression + working-directory: upstream/tee-output + run: python -m unittest discover -s tests -v + + - name: Verify Greg patch integrity + run: | + git -C upstream/tee-output diff --check + git -C upstream/tee-output diff --binary > artifacts/greg/tee-output-3.patch + test -s artifacts/greg/tee-output-3.patch + cat > artifacts/greg/receipt-${{ matrix.os }}-py${{ matrix.python }}.txt <> artifacts/greg/receipt-${{ matrix.os }}-py${{ matrix.python }}.txt + + - name: Upload Greg contributor kit + uses: actions/upload-artifact@v4 + with: + name: greg-tee-output-3-${{ matrix.os }}-py${{ matrix.python }}-${{ github.run_id }}-${{ github.run_attempt }} + path: artifacts/greg + if-no-files-found: error + + contributor-gate: + name: Contributor kit gate + if: always() + needs: + - boris-claude-code-action-1522 + - greg-tee-output-3 + runs-on: ubuntu-latest + steps: + - name: Enforce all candidate checks + env: + BORIS_RESULT: ${{ needs.boris-claude-code-action-1522.result }} + GREG_RESULT: ${{ needs.greg-tee-output-3.result }} + run: | + test "$BORIS_RESULT" = success + test "$GREG_RESULT" = success + echo "Both upstream contributor kits are exact-source, tested, and ready for fork-based PR publication." From 3b0bef3095e0eb6f287ac9e92d43453891f8b294 Mon Sep 17 00:00:00 2001 From: Aleksey Safonov <55020240+safal207@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:30:30 +0300 Subject: [PATCH 5/9] test: add READY relay and EOF-drain Greg candidate --- .../upstream-contributor-kits/apply_greg_3.py | 141 ++++++++++++++++-- 1 file changed, 129 insertions(+), 12 deletions(-) diff --git a/contrib/upstream-contributor-kits/apply_greg_3.py b/contrib/upstream-contributor-kits/apply_greg_3.py index 4ed61f58..716218c9 100644 --- a/contrib/upstream-contributor-kits/apply_greg_3.py +++ b/contrib/upstream-contributor-kits/apply_greg_3.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Apply the bounded candidate fix for gdb/tee-output#3. +# Apply the bounded READY-handshake + EOF-drain candidate for gdb/tee-output#3. # Every source replacement must match once against the pinned upstream revision. from __future__ import annotations @@ -17,6 +17,58 @@ def replace_once(path: Path, old: str, new: str) -> None: path.write_text(text.replace(old, new, 1), encoding="utf-8") +RELAY_FILE = r'''"""Small binary relay used by :mod:`tee_output`. + +The relay opens every output target before acknowledging readiness, then copies +stdin to the original stream and all targets until EOF. Exiting only after the +read loop finishes gives ``Tee.close()`` a concrete drain-completion boundary. +""" + +import argparse +import errno +import os + + +def copy_stream(paths, ready_fd): + outputs = [open(path, "ab", buffering=0) for path in paths] + try: + os.write(ready_fd, b"1") + finally: + os.close(ready_fd) + + try: + while True: + try: + chunk = os.read(0, 65536) + except OSError as exc: + # Closing a PTY master is reported as EIO on some platforms. + if exc.errno == errno.EIO: + break + raise + if not chunk: + break + + os.write(1, chunk) + for output in outputs: + output.write(chunk) + finally: + for output in outputs: + output.close() + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--ready-fd", type=int, required=True) + parser.add_argument("paths", nargs="+") + args = parser.parse_args() + copy_stream(args.paths, args.ready_fd) + + +if __name__ == "__main__": + main() +''' + + TEST_FILE = r'''import errno import os import pty @@ -91,11 +143,17 @@ def test_print_traceback_immediate_close_preserves_all_outputs(self): ) root = Path(tmp) - self.assertIn("stdout-marker", (root / "stdout.log").read_text()) - self.assertIn("stderr-marker", (root / "stderr.log").read_text()) + stdout_text = (root / "stdout.log").read_text() + stderr_text = (root / "stderr.log").read_text() combined = (root / "combined.log").read_text() + terminal_text = terminal.decode(errors="replace") + + self.assertIn("stdout-marker", stdout_text) + self.assertIn("stderr-marker", stderr_text) self.assertIn("stdout-marker", combined) self.assertIn("stderr-marker", combined) + self.assertIn("stdout-marker", terminal_text) + self.assertIn("stderr-marker", terminal_text) if __name__ == "__main__": @@ -144,6 +202,13 @@ def main() -> None: tee = repo / "tee_output/__init__.py" parent = repo / "bin/parent-lifetime" + relay = repo / "tee_output/_relay.py" + + replace_once( + tee, + "import os\nimport signal\n", + "import os\nimport select\nimport signal\n", + ) replace_once( tee, @@ -177,17 +242,15 @@ def _drain(self, stdout_pipe_proc, stderr_pipe_proc): ''', ''' def close(self): # Flush Python-level buffers before restoring the original descriptors. - # This is separate from draining the external tee processes below. sys.stdout.flush() sys.stderr.flush() self.pause() self._drain(self.stdout_pipe_proc, self.stderr_pipe_proc) def _drain(self, stdout_pipe_proc, stderr_pipe_proc): - # One sharp edge is that if you've spawned a subprocess with - # the redirected stdout/stderr, the tee processes will not die from EOF. - # Preserve bounded shutdown for that case, but do not interrupt the - # normal path before tee has had a chance to drain. + # Closing the writer delivers EOF to the bundled relay. Natural process + # completion is the drain acknowledgement; SIGINT is only a bounded + # fallback for inherited descriptors or a stuck reader. self._drain_one(stdout_pipe_proc) self._drain_one(stderr_pipe_proc) @@ -198,17 +261,13 @@ def _drain_one(pipe_proc): pipe, proc = pipe_proc pipe.close() - try: - # Closing the final writer delivers EOF. Let tee consume the - # buffered PTY/pipe tail and exit naturally before using signals. proc.wait(timeout=2) return except subprocess.TimeoutExpired: pass try: - # Fallback for inherited descriptors or a stuck reader stack. os.kill(proc.pid, signal.SIGINT) except ProcessLookupError: return @@ -216,6 +275,61 @@ def _drain_one(pipe_proc): ''', ) + replace_once( + tee, + ''' # TODO: fast exit + proc = subprocess.Popen( + ["parent-lifetime", "--term", "tee", "-a"] + list(to), + stdin=r, + start_new_session=True, + stderr=subprocess.DEVNULL, + stdout=stdout, + preexec_fn=set_ctty, + ) + r.close() + return w, proc +''', + ''' # The relay acknowledges only after every target has been opened. This + # distinguishes "process spawned" from "reader ready" and prevents the + # first write from racing external tee startup. + ready_r, ready_w = os.pipe() + proc = subprocess.Popen( + [ + "parent-lifetime", + "--term", + sys.executable, + "-m", + "tee_output._relay", + "--ready-fd", + str(ready_w), + ] + + list(to), + stdin=r, + start_new_session=True, + stderr=subprocess.DEVNULL, + stdout=stdout, + preexec_fn=set_ctty, + pass_fds=(ready_w,), + ) + r.close() + os.close(ready_w) + + ready, _, _ = select.select([ready_r], [], [], 2) + marker = os.read(ready_r, 1) if ready else b"" + os.close(ready_r) + if marker != b"1": + w.close() + try: + os.kill(proc.pid, signal.SIGINT) + except ProcessLookupError: + pass + proc.wait() + raise RuntimeError("tee relay did not become ready") + + return w, proc +''', + ) + replace_once( parent, ''' # Let the process die naturally @@ -239,6 +353,8 @@ def _drain_one(pipe_proc): ''', ) + relay.write_text(RELAY_FILE, encoding="utf-8") + tests = repo / "tests" tests.mkdir(exist_ok=True) (tests / "test_immediate_close.py").write_text(TEST_FILE, encoding="utf-8") @@ -254,6 +370,7 @@ def _drain_one(pipe_proc): "py_compile", str(tee), str(parent), + str(relay), str(tests / "test_immediate_close.py"), ], check=True, From c5729592eaff2c34d073d03b7b1e1ef76e4df160 Mon Sep 17 00:00:00 2001 From: Aleksey Safonov <55020240+safal207@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:31:47 +0300 Subject: [PATCH 6/9] ci: preserve Greg diagnostics on regression failure --- .../workflows/upstream-contributor-kits.yml | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/.github/workflows/upstream-contributor-kits.yml b/.github/workflows/upstream-contributor-kits.yml index 1759cde3..ce19b19e 100644 --- a/.github/workflows/upstream-contributor-kits.yml +++ b/.github/workflows/upstream-contributor-kits.yml @@ -123,7 +123,7 @@ jobs: with: python-version: ${{ matrix.python }} - - name: Apply EOF-before-SIGINT candidate + - name: Apply READY-handshake and EOF-drain candidate run: | python contrib/upstream-contributor-kits/apply_greg_3.py \ upstream/tee-output \ @@ -136,11 +136,20 @@ jobs: python -m pip install -e . - name: Run PTY immediate-close regression + id: regression + continue-on-error: true + shell: bash working-directory: upstream/tee-output - run: python -m unittest discover -s tests -v + run: | + mkdir -p ../../artifacts/greg + set -o pipefail + python -m unittest discover -s tests -v 2>&1 | \ + tee ../../artifacts/greg/regression-${{ matrix.os }}-py${{ matrix.python }}.log - name: Verify Greg patch integrity + if: always() run: | + mkdir -p artifacts/greg git -C upstream/tee-output diff --check git -C upstream/tee-output diff --binary > artifacts/greg/tee-output-3.patch test -s artifacts/greg/tee-output-3.patch @@ -150,18 +159,26 @@ jobs: upstream_sha=c41f8ff383200320b746e953e92709ae1b505a71 os=${{ matrix.os }} python=${{ matrix.python }} - contract=flush_pause_close_writer_wait_natural_then_sigint_fallback + regression_outcome=${{ steps.regression.outcome }} + contract=ready_handshake_bundled_relay_eof_drain_sigint_fallback trajectory=pty_print_traceback_immediate_close_10_rounds EOF shasum -a 256 artifacts/greg/tee-output-3.patch >> artifacts/greg/receipt-${{ matrix.os }}-py${{ matrix.python }}.txt - - name: Upload Greg contributor kit + - name: Upload Greg contributor kit and diagnostics + if: always() uses: actions/upload-artifact@v4 with: name: greg-tee-output-3-${{ matrix.os }}-py${{ matrix.python }}-${{ github.run_id }}-${{ github.run_attempt }} path: artifacts/greg if-no-files-found: error + - name: Enforce Greg regression result + if: always() + env: + REGRESSION_OUTCOME: ${{ steps.regression.outcome }} + run: test "$REGRESSION_OUTCOME" = success + contributor-gate: name: Contributor kit gate if: always() From ef63f2ba32ef0e4aaa236746f661f87d3bbcbd9f Mon Sep 17 00:00:00 2001 From: Aleksey Safonov <55020240+safal207@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:35:28 +0300 Subject: [PATCH 7/9] test: add ordered in-stream drain acknowledgement --- .../upstream-contributor-kits/apply_greg_3.py | 139 ++++++++++++------ 1 file changed, 96 insertions(+), 43 deletions(-) diff --git a/contrib/upstream-contributor-kits/apply_greg_3.py b/contrib/upstream-contributor-kits/apply_greg_3.py index 716218c9..794d7efc 100644 --- a/contrib/upstream-contributor-kits/apply_greg_3.py +++ b/contrib/upstream-contributor-kits/apply_greg_3.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Apply the bounded READY-handshake + EOF-drain candidate for gdb/tee-output#3. +# Apply the bounded READY + in-stream DRAIN-ACK candidate for gdb/tee-output#3. # Every source replacement must match once against the pinned upstream revision. from __future__ import annotations @@ -17,26 +17,36 @@ def replace_once(path: Path, old: str, new: str) -> None: path.write_text(text.replace(old, new, 1), encoding="utf-8") -RELAY_FILE = r'''"""Small binary relay used by :mod:`tee_output`. - -The relay opens every output target before acknowledging readiness, then copies -stdin to the original stream and all targets until EOF. Exiting only after the -read loop finishes gives ``Tee.close()`` a concrete drain-completion boundary. -""" +RELAY_FILE = r'''"""Binary relay with explicit readiness and drain acknowledgements.""" import argparse import errno import os -def copy_stream(paths, ready_fd): +def write_all(fd, data): + view = memoryview(data) + while view: + written = os.write(fd, view) + view = view[written:] + + +def emit(outputs, data): + if not data: + return + write_all(1, data) + for output in outputs: + write_all(output.fileno(), data) + + +def copy_stream(paths, status_fd, drain_token): outputs = [open(path, "ab", buffering=0) for path in paths] + pending = b"" + drain_acknowledged = False try: - os.write(ready_fd, b"1") - finally: - os.close(ready_fd) + # READY means every output target is open and the relay can read stdin. + write_all(status_fd, b"R") - try: while True: try: chunk = os.read(0, 65536) @@ -48,20 +58,39 @@ def copy_stream(paths, ready_fd): if not chunk: break - os.write(1, chunk) - for output in outputs: - output.write(chunk) + pending += chunk + marker_index = pending.find(drain_token) + if marker_index >= 0: + emit(outputs, pending[:marker_index]) + pending = pending[marker_index + len(drain_token) :] + write_all(status_fd, b"D") + drain_acknowledged = True + continue + + # Keep enough suffix bytes to detect a token split across reads. + keep = max(0, len(drain_token) - 1) + if len(pending) > keep: + emit(outputs, pending[:-keep] if keep else pending) + pending = pending[-keep:] if keep else b"" + + emit(outputs, pending) + if not drain_acknowledged: + # EOF without a sentinel is valid for callers that never close via + # Tee.close(), but no false DRAIN acknowledgement is emitted. + pass finally: for output in outputs: output.close() + os.close(status_fd) def main(): parser = argparse.ArgumentParser() - parser.add_argument("--ready-fd", type=int, required=True) + parser.add_argument("--status-fd", type=int, required=True) + parser.add_argument("--drain-token", required=True) parser.add_argument("paths", nargs="+") args = parser.parse_args() - copy_stream(args.paths, args.ready_fd) + copy_stream(args.paths, args.status_fd, bytes.fromhex(args.drain_token)) if __name__ == "__main__": @@ -120,7 +149,7 @@ def read_terminal(master): class ImmediateCloseTest(unittest.TestCase): def test_print_traceback_immediate_close_preserves_all_outputs(self): - for round_id in range(10): + for round_id in range(25): with self.subTest(round=round_id), tempfile.TemporaryDirectory() as tmp: master, slave = pty.openpty() try: @@ -241,16 +270,38 @@ def _drain(self, stdout_pipe_proc, stderr_pipe_proc): proc.wait() ''', ''' def close(self): - # Flush Python-level buffers before restoring the original descriptors. + # Flush Python-level buffers, then place an in-stream sentinel after + # every prior byte. The relay acknowledges only after persisting all + # bytes before that sentinel. sys.stdout.flush() sys.stderr.flush() + self._request_drain(self.stdout_pipe_proc, sys.stdout.fileno()) + self._request_drain(self.stderr_pipe_proc, sys.stderr.fileno()) + stdout_drained = self._wait_for_drain(self.stdout_pipe_proc) + stderr_drained = self._wait_for_drain(self.stderr_pipe_proc) + self.pause() self._drain(self.stdout_pipe_proc, self.stderr_pipe_proc) + if not stdout_drained or not stderr_drained: + raise RuntimeError("tee relay did not acknowledge drain") + + @staticmethod + def _request_drain(pipe_proc, fd): + if pipe_proc is not None: + os.write(fd, pipe_proc[3]) + + @staticmethod + def _wait_for_drain(pipe_proc): + if pipe_proc is None: + return True + status_r = pipe_proc[2] + ready, _, _ = select.select([status_r], [], [], 2) + return bool(ready and os.read(status_r, 1) == b"D") + def _drain(self, stdout_pipe_proc, stderr_pipe_proc): # Closing the writer delivers EOF to the bundled relay. Natural process - # completion is the drain acknowledgement; SIGINT is only a bounded - # fallback for inherited descriptors or a stuck reader. + # completion is the final boundary; SIGINT remains a bounded fallback. self._drain_one(stdout_pipe_proc) self._drain_one(stderr_pipe_proc) @@ -259,19 +310,18 @@ def _drain_one(pipe_proc): if pipe_proc is None: return - pipe, proc = pipe_proc + pipe, proc, status_r, _ = pipe_proc pipe.close() try: proc.wait(timeout=2) - return except subprocess.TimeoutExpired: - pass - - try: - os.kill(proc.pid, signal.SIGINT) - except ProcessLookupError: - return - proc.wait() + try: + os.kill(proc.pid, signal.SIGINT) + except ProcessLookupError: + pass + proc.wait() + finally: + os.close(status_r) ''', ) @@ -289,10 +339,11 @@ def _drain_one(pipe_proc): r.close() return w, proc ''', - ''' # The relay acknowledges only after every target has been opened. This - # distinguishes "process spawned" from "reader ready" and prevents the - # first write from racing external tee startup. - ready_r, ready_w = os.pipe() + ''' # The relay opens every output target before READY. A random sentinel + # later provides an ordered post-write drain acknowledgement on the same + # PTY/pipe byte stream without leaking control bytes to the output. + status_r, status_w = os.pipe() + drain_token = b"\\x00tee-output-drain:" + os.urandom(16) + b"\\x00" proc = subprocess.Popen( [ "parent-lifetime", @@ -300,8 +351,10 @@ def _drain_one(pipe_proc): sys.executable, "-m", "tee_output._relay", - "--ready-fd", - str(ready_w), + "--status-fd", + str(status_w), + "--drain-token", + drain_token.hex(), ] + list(to), stdin=r, @@ -309,16 +362,16 @@ def _drain_one(pipe_proc): stderr=subprocess.DEVNULL, stdout=stdout, preexec_fn=set_ctty, - pass_fds=(ready_w,), + pass_fds=(status_w,), ) r.close() - os.close(ready_w) + os.close(status_w) - ready, _, _ = select.select([ready_r], [], [], 2) - marker = os.read(ready_r, 1) if ready else b"" - os.close(ready_r) - if marker != b"1": + ready, _, _ = select.select([status_r], [], [], 2) + marker = os.read(status_r, 1) if ready else b"" + if marker != b"R": w.close() + os.close(status_r) try: os.kill(proc.pid, signal.SIGINT) except ProcessLookupError: @@ -326,7 +379,7 @@ def _drain_one(pipe_proc): proc.wait() raise RuntimeError("tee relay did not become ready") - return w, proc + return w, proc, status_r, drain_token ''', ) From 24a8eeae331d82f194e69bdf26b5894bf3a12428 Mon Sep 17 00:00:00 2001 From: Aleksey Safonov <55020240+safal207@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:36:32 +0300 Subject: [PATCH 8/9] docs: describe ordered drain acknowledgement protocol --- contrib/upstream-contributor-kits/README.md | 41 ++++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/contrib/upstream-contributor-kits/README.md b/contrib/upstream-contributor-kits/README.md index 06cdddc4..caf962ee 100644 --- a/contrib/upstream-contributor-kits/README.md +++ b/contrib/upstream-contributor-kits/README.md @@ -43,30 +43,51 @@ gdb/tee-output c41f8ff383200320b746e953e92709ae1b505a71 ``` -Problem boundary: +Observed lifecycle boundary: ```text -write end closes - -> SIGINT is sent immediately to parent-lifetime - -> underlying tee may be interrupted before startup/tail drain completes +process spawned is not reader ready +reader ready is not output drained +closing a PTY master can discard an unread tail ``` -Candidate contract: +The first shutdown-only candidate failed on all four OS/Python coordinates. A +READY-only bundled relay then passed most runs but still produced an intermittent +empty stdout log on macOS. The final candidate therefore uses two ordered +acknowledgements: ```text -flush Python streams -close writer and deliver EOF -wait for natural reader completion -use SIGINT only as a bounded fallback +open every output target + -> READY acknowledgement + -> permit normal writes + -> flush Python streams + -> append a random sentinel to the same PTY/pipe byte stream + -> relay persists every byte before the sentinel + -> relay removes the sentinel from user-visible output + -> DRAIN acknowledgement on a separate status pipe + -> restore descriptors and close the writer + -> wait for natural process completion + -> use SIGINT only as a bounded fallback ``` +Because the drain sentinel follows user bytes in the same stream, the +acknowledgement establishes ordering rather than relying on a sleep, file +existence, or process liveness. + The generated patch adds a PTY regression test for: ```text print -> traceback -> immediate Tee.close() ``` -and runs it on Linux and macOS. +Validation matrix: + +```text +Linux + macOS +Python 3.11 + 3.13 +25 rounds per coordinate +100 total immediate-close trajectories +``` ## Authority boundary From 68c8a49a46505521d8c31ae2ff73a4515e6c0942 Mon Sep 17 00:00:00 2001 From: Aleksey Safonov <55020240+safal207@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:38:36 +0300 Subject: [PATCH 9/9] ci: bind Greg receipt to 25-round sentinel contract --- .github/workflows/upstream-contributor-kits.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/upstream-contributor-kits.yml b/.github/workflows/upstream-contributor-kits.yml index ce19b19e..8529cfe3 100644 --- a/.github/workflows/upstream-contributor-kits.yml +++ b/.github/workflows/upstream-contributor-kits.yml @@ -123,7 +123,7 @@ jobs: with: python-version: ${{ matrix.python }} - - name: Apply READY-handshake and EOF-drain candidate + - name: Apply READY and in-stream DRAIN-ACK candidate run: | python contrib/upstream-contributor-kits/apply_greg_3.py \ upstream/tee-output \ @@ -160,8 +160,8 @@ jobs: os=${{ matrix.os }} python=${{ matrix.python }} regression_outcome=${{ steps.regression.outcome }} - contract=ready_handshake_bundled_relay_eof_drain_sigint_fallback - trajectory=pty_print_traceback_immediate_close_10_rounds + contract=ready_handshake_in_stream_sentinel_drain_ack_natural_exit_sigint_fallback + trajectory=pty_print_traceback_immediate_close_25_rounds EOF shasum -a 256 artifacts/greg/tee-output-3.patch >> artifacts/greg/receipt-${{ matrix.os }}-py${{ matrix.python }}.txt