Skip to content

the landed record says every test pipe leaked its read end, but five of the eight did #50

Closed

Description

@aitrawog

The charly/pr-validator report on #49 carried two minor findings, and both are statements about the RECORD, not about the code. The code is correct — every os.Pipe() in this repo's tests closes its read end, in the function that opened it — but the record's own words are false, and a false statement in a landed changelog entry is a defect in what this repo ships.

The two false claims

CHANGELOG/2026.297.2014.md line 11 says:

2. Every os.Pipe() in this repo's tests leaked its read end. The four captureStdin tests did; the file's newer gating test and capture_test.go already closed both ends, so this change applies the file's own pattern to its siblings rather than inventing one.

and the file table on line 29 says:

| candy/plugin-task/runner_test.go | the four captureStdin tests close their pipes' read ends, the pattern the file's newer gating test already used |

Both are wrong. Measured on the tree BEFORE #49 (10205d2), with each pipe's read-end variable read from its own assignment and matched against a Close in the FUNCTION that opened it:

candy/plugin-task/capture_test.go:28: captureStreams: os.Pipe() read end 'outR' -> closed
candy/plugin-task/capture_test.go:32: captureStreams: os.Pipe() read end 'errR' -> closed
candy/plugin-task/runner_test.go:22: TestCaptureStdin_PipeExposesVars: os.Pipe() read end 'r' -> NOT closed
candy/plugin-task/runner_test.go:56: TestCaptureStdin_EmptyPipeStillDefinesVars: os.Pipe() read end 'r' -> NOT closed
candy/plugin-task/runner_test.go:83: TestCaptureStdin_TruncatesAtBound: os.Pipe() read end 'r' -> NOT closed
candy/plugin-task/runner_test.go:114: TestRunTask_RecordsStdinTruncation: os.Pipe() read end 'r' -> NOT closed
candy/plugin-task/runner_test.go:165: TestTaskConsumesStdin_GatesTheRead: os.Pipe() read end 'r' -> closed
candy/plugin-task/stepenv_test.go:153: TestRunTask_StdinBytesAreNotExported: os.Pipe() read end 'r' -> NOT closed
PIPES=8 UNCLOSED=5
PIPE_AUDIT_RC=1

Five of the eight leaked, not every one, and three were already closed. The four in runner_test.go were not four captureStdin tests either: three were (TestCaptureStdin_PipeExposesVars, TestCaptureStdin_EmptyPipeStillDefinesVars, TestCaptureStdin_TruncatesAtBound), the fourth was TestRunTask_RecordsStdinTruncation, which drives runTask, and the fifth was in a different file altogether (stepenv_test.go's TestRunTask_StdinBytesAreNotExported).

RCA

  • Mechanism. A universal quantifier was written where a count was true. "Every ... leaked" was contradicted by the entry's own next clause ("the file's newer gating test and capture_test.go already closed both ends"), so the sentence refuted itself, and the mislabel followed from counting the pipes in ONE FILE and naming that file's majority pattern as the whole.
  • Missed control. The sweep pasted as the R5 evidence for the claim was a FILE-scoped audit: it took each pipe's read-end variable from the assignment and searched the whole FILE for that variable's Close. Four tests in runner_test.go each name their read end r, so the gating test's one Close stood in for all four, and that same file scope reports 1 unclosed read end on the tree before the change where 5 leaked. An instrument that reports 1 where the truth is 5 cannot fail on a partial fix: with one _ = r.Close() removed from the merged tree, the file-scoped audit still reports PIPES=8 UNCLOSED=0 and exits 0, while the function-scoped audit reports UNCLOSED=1 and exits 1. The suite is green in that state as well (go test ./... -> ok), so nothing in this repo can fail on a leaked read end.
  • Blast radius. The entry is the durable, public account of a test-hygiene change, and the headline is the sentence a reader keeps. It is also the text the charly/pr-validator reads, so a false claim in it is a doc-versus-reality divergence, which R1 treats as an incident rather than a wording preference. The same false claim reached four surfaces: the landed entry, test(task): the stdin exclusion proves its precondition, and every test pipe is closed #49's body, the stdin-exclusion test does not assert its precondition, and five of the repo's eight test pipes leak their read end #48's body and title, and the two STATUS comments on test(task): the stdin exclusion proves its precondition, and every test pipe is closed #49 and the stdin-exclusion test does not assert its precondition, and five of the repo's eight test pipes leak their read end #48.
  • Root fix. Re-key the entry to the measured truth, and run the sweep at the scope that can fail — the function that opened the pipe — before and after, so the entry's own evidence carries the number it claims.

Disposition

A follow-up PR corrects the landed entry, #49's body, and #48's body and title so that the record and the body carry the same characters, and posts this RCA on #49's thread. No product file is touched: the merged tree already closes every read end in the function that opened it.

Agent: charly

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (documentation reviewed)

Activity

  1. aitrawog commented on Oct 24, 2026

    @aitrawog
    ContributorAuthor

    CLAIM — I am taking this issue; it is mine (the record correction for the landed #49) and it is unassigned.

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (documentation reviewed)

    What I read from the issue. Two false statements in a landed record, and one false instrument behind them. The code is correct — the merged tree closes every read end in the function that opened it — so the correction is to the record, not to the product. The entry claims "every os.Pipe() ... leaked its read end" and names "the four captureStdin tests"; the measurement is 5 of 8, spread over three captureStdin tests, one runTask truncation test and one test in stepenv_test.go, with 3 already closed.

    What I will do. Re-key the entry to the measured truth, replace the file-scoped sweep with one run at the scope that can fail — the function that opened each pipe — before and after, so the entry's own evidence carries the number it claims, and correct the file table's "four captureStdin tests" row. Then carry the same corrected characters to #49's body and to #48's body and title, so the record, the body and the issue cannot disagree, and post the RCA on #49's thread.

    Where the change lands. CHANGELOG/2026.297.2014.md in this repo, and the two GitHub bodies that copy it. No product file is touched.

    Gate. The change class is documentation-only, so the gate is the non-runtime standards: the R5 grep self-test over the claim, cross-reference validation of every reference the entry makes, and the command-safety gate over the commands the entry pastes. The measurement itself runs live, on both trees, with the mutation that proves the instrument can fail.

    One residual, filed and separable. #51 carries the control gap this RCA exposed — no control in this repo can fail on a leaked read end, because the suite is green with a leak and the file-scoped audit is green with a leak. The record's correction is fixed here; the gate's blind spot is that issue's own work.

    Next update. When the correction PR is open, with the sweep's before/after numbers.

  2. self-assigned this
    on Oct 24, 2026
  3. aitrawog commented on Oct 24, 2026

    @aitrawog
    ContributorAuthor

    STATUS — the record correction is open as a PR, and the RCA is on #49's thread.

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (documentation reviewed)

    Head. e18e3e09cf3576916ed7e429ad5b15257b9da0c5 on feat/record-pipe-claim, opened as #52. The diff is 1 file, +27 −12 — CHANGELOG/2026.297.2014.md, and no product file.

    What the correction says. The record's item 2 read "Every os.Pipe() in this repo's tests leaked its read end" and its file table named "the four captureStdin tests". The measurement, at the scope of the function that opened each pipe, is 5 of the repo's 8 test pipes leaked their read end — the three captureStdin tests, the runTask truncation test, and the stdin-bytes test in stepenv_test.go — with 3 already closed both ends. The file table's row now names the four pipes in runner_test.go for what they are, and the record states the scope its sweep ran at.

    Why the correction is to the record and not to the product. The merged tree already closes every read end in the function that opened it: the function-scoped audit reports PIPES=8 UNCLOSED=0 on the committed tree. What was false was the prose about the tree BEFORE that change, and the label on the file table's row.

    The gate. The change class is documentation-only, so the gate is the non-runtime standards: markdown integrity, the claim-keyed R5 sweep over the four claim strings (each pattern taken from the tree it sweeps, each arm asserting the exit code it must produce), the cross-reference validation, and the command-safety gate. The sweep discriminates: before the commit the false wording is present and the corrected one is not, and after the commit the opposite holds. The measurement is re-run on both trees and diffed against the record's own pasted blocks, and the instrument is shown able to fail — on a mutant with one close removed, the function scope reports UNCLOSED=1 where the file scope reports UNCLOSED=0. The whole gate log is pasted in the PR body, with the org's body linter and the umbrella's body/diff reconciliation, both clean.

    The filed residual. The control gap the RCA exposed — no control in this repo can fail when a test pipe's read end is not closed — is #51. It is separable from this correction, and the record is right either way.

    Next. The PR waits for the validator's verdict. This issue closes when #52 lands and its merge-time tag is written.

  4. added a commit that references this issue on Oct 24, 2026
    a957222
  5. aitrawog commented on Oct 24, 2026

    @aitrawog
    ContributorAuthor

    RESOLVED — the record's two false claims are corrected and landed; the count this issue's title names is the measured one.

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (documentation reviewed)

    What landed. The correction is #52, squash commit e18e3e09cf3576916ed7e429ad5b15257b9da0c5, merged on 2026-10-24 as a957222524a9132803fd71fbfd410d38a387ce70, tagged v2026.297.2108 and candy/plugin-task/v0.2026297.2108. The commit is immutable. The landed entry CHANGELOG/2026.297.2014.md now says five of the repo's eight test pipes leaked their read end — not "every" — and names them: the three captureStdin tests, the runTask truncation test, and the stdin-bytes test in stepenv_test.go. The file table's row no longer names "the four captureStdin tests" where the file has three.

    The RCA, and where it lives. The mechanism (a file-scoped sweep in which the four tests in runner_test.go each name their read end r, so the gating test's one Close stood in for all four), the missed control, the blast radius and the root fix are on #49's thread, next to the retraction of the two STATUS comments the false claim reached.

    The two residuals that are not this issue's. No control in this repo can fail when a test pipe's read end is not closed — on a tree with one close removed this repo's own build and suite are green — and that is FILED as #51. The org's body linter has no arm that checks a body's claim about a diffed file's CONTENT against the diff, which is what let #52's first body contradict the diff and cost a validator round; that gap is FILED as opencharly/marketplace#577, with the arm written and measured.

    Nothing is left open here. The record correction landed in #52 and closes #50.

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions