Skip to content

fix(plugin): wait for Codex stdin probe publication - #2094

Merged
dnlrsls merged 1 commit into
Gentleman-Programming:mainfrom
dnlrsls:fix/2072-codex-stdin-publication
Oct 24, 2026
Merged

dnlrsls merged 1 commit into
Gentleman-Programming:mainfrom
dnlrsls:fix/2072-codex-stdin-publication

Conversation

@dnlrsls

@dnlrsls dnlrsls commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

🔗 Linked Issue

Closes #2072

Lifecycle Evidence

  • Linked issue is approved and assigned.
  • Bug evidence includes exact source SHA, deterministic RED, root cause and focused regression tests.
  • Evidence is recorded below; no separate design document is required.
  • Current issue-first requirements apply before merge.

🏷️ PR Type

  • type:bug — Bug fix
  • type:feature — New feature
  • type:question — Question requiring tracked work
  • type:docs — Documentation only
  • type:refactor — Code refactoring
  • type:chore — Maintenance/tooling
  • type:breaking-change — Breaking change

📝 Summary

  • Treat a successfully read but empty stdin-probe marker as unpublished, rather than asserting EOF prematurely.
  • Hold the fixture between file creation and printf publication until the reader observes that window, making the regression deterministic.
  • Preserve exact EOF/data validation, the open parent stdin pipe, foreground exit check and existing one-second deadlines. No production hook changes.

📂 Changes

File Change
plugin/codex_unix_user_prompt_test.go Reader-controlled publication barrier, pending-empty result handling, and table coverage of empty/eof/data results.

🧪 Test Plan

  • Deterministic RED on source snapshot 3e0642074b8169419cb411ef0426d91145c8e462: hold publication after opening the marker while preserving the old validation; focused stdin test failed with detached curl stdin = "", want EOF from /dev/null. Empty-result table case failed; EOF/data controls passed.
  • Focused GREEN: go test ./plugin -run '^TestCodex(PromptStdinProbeResult|UnixUserPromptSubmitDetachesPromptPersistenceStdin)$' -count=1 -v — PASS, package 15.318s.
  • Affected package: go test ./plugin -count=1 — PASS, 79.060s.
  • Structural check: git diff --check — PASS; only this test file is committed.

Latest local GREEN ran on Linux/WSL, Go 1.25.10, native short TMPDIR=/tmp/engram-2072, using an uninstrumented clean snapshot of merged main aec0f4f0f6fa57aad594c9abf45fb7e0be67f68c plus only this exact test file. Its SHA256 matched the local candidate: 3f41552dd1460965cf4f9314abb74bdf7729cf56e83ea0b8f7e6f1fc0dc72400.

Earlier local evidence is not erased: Windows baseline failed with prompt persistence did not finish the stdin probe, a missing-marker symptom not attributed to this empty-publication race. An earlier Linux package run with long mounted TMPDIR failed Unix socket binds and Claude fixture ancestry checks; socket checks passed with native TMPDIR and the Claude gap was reported as #2089, fixed by merged #2092. The fresh full plugin PASS above includes that merged fix. No fresh Windows candidate test, full module suite, E2E, lint or hosted CI result is claimed here.

🤖 Automated Checks

GitHub checks are pending until they actually run on this PR head. Local Go plugin results are not the JavaScript Pi Plugin Tests CI context.

Check Status
Check Issue Reference Pending
Check Issue Has status:approved Pending
Check PR Has type:* Label Pending CI; type:bug applied on creation
Check PR Has No Transient Artifacts Pending
Unit Tests Pending
E2E Tests Pending
Plugin Tests Pending
Lint Pending
Windows Setup Test Pending
Cloud Sync Wrapper Tests (Windows) Pending

✅ Contributor Checklist

  • Linked an approved, assigned issue above.
  • Added exactly one type:* label: type:bug.
  • Recorded actual focused regression and affected-package results.
  • Recorded structural checks and identified missing broad/platform CI evidence.
  • Docs: N/A, test-only change; no user-facing command or production behavior changed.
  • Conventional Commit format.
  • No Co-Authored-By trailers.
  • Committed path complies with the transient-artifact policy; local .tmp/ tooling is excluded.

💬 Notes for Reviewers

The root cause is shell redirection creating/truncating the file before printf writes its contents. File existence/read success is not publication completion. Invalid nonempty results still fail with the original exact error. The fixture releases its barrier on test failure and stops waiting if its temporary directory disappears. No timeout increase, production parser/helper change, stdin guard relaxation or Windows missing-marker fix is included.

Risk: none — test-only behavior, no shared production code changed. Native assessment was unavailable because local untracked setup artifacts required a declaration; its RDD-off plan required writer self-verification only. No native review approval or merge readiness is claimed.

Summary by CodeRabbit

  • Tests
    • Expanded validation of prompt input handling across detached-input, inherited-input, and incomplete-probe scenarios. The checks now confirm whether input reaches the prompt process, helping catch regressions in how prompt input is handled.

Treat an empty probe marker as pending rather than a completed EOF result. Hold fixture publication behind a reader-controlled barrier to reproduce the empty-file window deterministically, retaining data rejection, open parent stdin and existing deadlines.

Verified focused regressions and go test ./plugin -count=1 on Linux with Go 1.25.10 against main aec0f4f plus this exact test candidate. No production hook changes. Refs: Gentleman-Programming#2072.
@dnlrsls dnlrsls added the type:bug Bug fix label Oct 24, 2026
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between aec0f4f and 2db5c64.

Files selected for processing: 1
  • plugin/codex_unix_user_prompt_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.

Walkthrough

The stdin-detachment test now holds the mock prompt process until the test releases it. The test waits for probe publication, then checks whether stdin produced EOF or data. New helper tests cover unpublished, EOF, and data results.

Changes: Codex stdin probe test

Layer / File(s) Summary
Probe publication and result validation
plugin/codex_unix_user_prompt_test.go
The mock curl process waits for a release signal before publishing whether stdin provided data or EOF. The test verifies the marker is initially empty, releases the probe, and checks its result. Helper tests cover an unpublished marker, EOF, and data.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: gentleman-programming

Merge Risk: Minimal · up to 2db5c

The stdin-detachment test retains exact result validation. No identified issue prevents merging after normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "fix(plugin): wait for Codex stdin probe publication" matches the main change. The test now treats an empty stdin-probe marker as pending and waits for publication before validating the resu…
Linked Issues check Passed Issue #2072 requires the test to distinguish an empty marker from a completed probe, preserve the detached-stdin EOF assertion, keep parent input open, verify adapter exit, and retain the existing dea…
Out of Scope Changes check Passed The whole pull request changes only plugin/codex_unix_user_prompt_test.go. The changes add synchronization and regression coverage for the marker publication race in issue #2072. The helper tests an…
Full details: Title check

Explanation

The title "fix(plugin): wait for Codex stdin probe publication" matches the main change. The test now treats an empty stdin-probe marker as pending and waits for publication before validating the result, and a new helper (TestCodexPromptStdinProbeResult) covers the unpublished, EOF, and inherited-data cases. The title names the plugin, the Codex stdin probe, and the publication wait. It is specific, concise, and uses the conventional commit form. No search was needed for this check.

Full details: Linked Issues check

Explanation

Issue #2072 requires the test to distinguish an empty marker from a completed probe, preserve the detached-stdin EOF assertion, keep parent input open, verify adapter exit, and retain the existing deadlines. The changed test uses codexPromptStdinProbeResult to treat an empty marker as pending and validates exact eof and data results. The release file creates a deterministic publication barrier. The test still uses the stdin pipe, checks adapter exit, and keeps the one-second polling and exit deadlines. Table tests cover unpublished, EOF, and data results. No production hook code changed.

Full details: Out of Scope Changes check

Explanation

The whole pull request changes only plugin/codex_unix_user_prompt_test.go. The changes add synchronization and regression coverage for the marker publication race in issue #2072. The helper tests and fixture cleanup support that objective. No unrelated production behavior, timeout relaxation, stdin assertion removal, or Windows missing-marker fix is included.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

@dnlrsls
dnlrsls added this pull request to the merge queue Oct 24, 2026
Merged via the queue into Gentleman-Programming:main with commit fcd9301 Oct 24, 2026
39 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(tests): Codex stdin probe reads marker before its contents are published

1 participant