Skip to content

fix(disk-hygiene): flag PowerShell >> append redirection like > - #2676

Merged
kyle-sexton merged 1 commit into
mainfrom
cursor/fix-disk-hygiene-ps-append-redirect-f8b0
Aug 15, 2026
Merged

fix(disk-hygiene): flag PowerShell >> append redirection like >#2676
kyle-sexton merged 1 commit into
mainfrom
cursor/fix-disk-hygiene-ps-append-redirect-f8b0

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Closes #2675

Summary

_POWERSHELL_OUTPUT_REDIRECT matched neither character of a >> pair, so <cmd> >> append.txt wrote a file with no prompt while the same command with > prompted.

Fix

Verification

Focused GuardTests for append redirects and stream merges pass locally.

Related

Refs #2615 / #2627 / #2671 — stream-merge and $null-discard carve-outs that must stay narrow.

@cursor

cursor Bot commented Aug 15, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c101954d05

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/disk-hygiene/skills/clean/scripts/destructive_guard.py Outdated
@cursor
cursor Bot force-pushed the cursor/fix-disk-hygiene-ps-append-redirect-f8b0 branch from c101954 to 0ac066f Compare August 15, 2026 05:05
@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 15, 2026 05:05
The single-`>` redirect pattern rejected both characters of a `>>` pair, so
append-to-file commands ran without a prompt. Match append explicitly
(`>>` / `2>>` / `*>>`), keep `>> $null` silent with a real token terminator
after `$null`, and cover punctuation continuations with a regression test.
Version 0.20.5 sits above main tip 0.20.4 from #2671.

Closes #2675

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/fix-disk-hygiene-ps-append-redirect-f8b0 branch from 2b7d8bb to 1f4ca35 Compare August 15, 2026 05:10
@claude

claude Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cursor[bot]'s task in 4m 34s —— View job


Code review — /review:code-review

  • Gather context (PR diff, CLAUDE.md, REVIEW.md)
  • Review changed files for correctness/maintainability
  • Post inline findings (if any)
  • Final summary

No blocking or high-signal findings. This is a well-scoped, well-tested fix for a real bug (>> structurally invisible to _POWERSHELL_OUTPUT_REDIRECT, so append-redirect commands bypassed the safety prompt).

What I checked

  • Manually traced _POWERSHELL_APPEND_REDIRECT (destructive_guard.py) against the new/old regex boundary, the $null-discard carve-out, terminator handling, case-insensitivity, and interaction with _POWERSHELL_OUTPUT_REDIRECT on the same >> pair — confirmed the two regexes cleanly partition the input with no double-match and no gap.
  • Independently re-verified with a second, adversarial pass (a separate agent instructed to actively look for a false negative — a real file write that slips past undetected) against multi-redirect commands, non-terminator punctuation after $null (:, ,, backtick, digits), arbitrary stream numbers (3>>, 6>>, etc.), and case variants. No false negative found; the only "loose" behavior is the intentional, tested conservative over-flagging of punctuation continuations after $null (same documented design as the existing single-> carve-out from fix(disk-hygiene): stop flagging PowerShell $null discards as file redirection #2671).
  • Confirmed the $null-exemption gap the Codex review flagged (missing token-terminator requirement, allowing >> $null/tmp/output.txt to slip through) is already fixed in this PR's current head (1f4ca35) via the (?=[\s;|)}]|$) terminator lookahead, and is covered by test_powershell_append_redirects_are_file_writes (>>$null/out.txt, 2>>$null\evil.ps1 cases).
  • Version bump (0.20.40.20.5) and CHANGELOG entry are consistent with the code changes and prior versioning (fix(disk-hygiene): stop flagging PowerShell $null discards as file redirection #2671 was 0.20.4).
  • I don't have a Python execution tool available in this lane, so I could not run test_hygiene.py myself — the above is manual regex tracing plus an independent agent's adversarial trace, not an executed test run. The PR description's "Focused GuardTests ... pass locally" claim is author-reported and unverified by me.

Minor, non-blocking observation

test_powershell_append_redirects_are_file_writes (test_hygiene.py:5607) covers >>$null/out.txt and 2>>$null\evil.ps1 as punctuation-continuation cases, but doesn't mirror the full punctuation set the analogous single-> test covers (:altstream, ,x — test_hygiene.py:5560-5567). Not a bug — the shared regex handles these identically — just a smaller parity gap in test breadth. Not blocking.

@github-actions

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@kyle-sexton
kyle-sexton merged commit 1a61290 into main Aug 15, 2026
40 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/fix-disk-hygiene-ps-append-redirect-f8b0 branch August 15, 2026 05:17
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
Re-pinning `_FIND_SIDE_EFFECT_PRIMARIES` and `MERGED_PR_GRAPHQL_ALIAS_PAGE`
from bare identifiers to their definition lines closed only one direction. A
definition line occurs exactly once and a use-site-only re-land cannot contain
it, so the use-only false green is gone -- but the inverse opened in its place:
a re-land that restores the definition and never reconnects the use satisfies
the row while the guard stays disconnected.

So each constant is now TWO marker rows, its definition and its only use, and
both must hold. A definition-only re-land passes the definition row and fails
the use row; a use-only re-land does the reverse.

Measured for both added literals, and re-measured independently for all seven
marker rows now in the corpus: each occurs exactly once in the file it binds
to on the working tree and at origin/main, zero times in that path at its
recorded incident sha, and at least once at that sha's first parent.

Also corrects the note above the 9239f15 row. It credited #2640 with the
destructive_guard.py restore; #2640 (6f0a311) changes no file under
plugins/disk-hygiene, both markers are absent at the only two intervening
commits that touched the file (#2671, #2676), and both first appear at
de26c27 (#2706). The #2640 credit on the cc58cbc row is correct and is
left alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LwdkpWf6bptu3AqTMoeg2H
kyle-sexton added a commit that referenced this pull request Aug 17, 2026
…two loose markers (#2930)

## What

Two related fixes to the silent-revert canary, both about claims that do
not hold: four comment claims that no longer reproduce, and two `marker`
literals that bind more loosely than the corpus prose implies.

## Part 1 — four comment claims re-derived (#2917)

None is executable, so nothing failed — which is the problem: they read
as measured, so the next person to touch the calibration would have
built on them. Each is **rewritten to a claim that stays true**, not
renumbered to a figure that rots again. That follows the standard set in
#2847, which removed a "fires roughly once a month" claim rather than
renumbering it, because the corpus measures a burst and no honest
per-month rate exists.

1. **"1527-commit history", stated twice.** Reproduced at no named
endpoint — 1546 at `738791c45`, 1549 shortly after. Now "every
first-parent commit of main". The result the pin rests on (`Revert "` 0,
`revert:` 1) is unchanged and now leads the sentence.
2. **"the replay exited 0 for 31h28m."** Credited the replay with a
silence it was not present for. 31h28m is the **content-absence**
window; this canary merged part-way through it at `7b47d2253`, 6h13m
before the restore. The passage now separates the two windows and states
what was true over the tail: the `f603880da` row reproduced the removal
exactly as recorded while the content was still gone. The row was never
wrong about what it asserted — it asserted the other question.
3. **The repo-wide-grep counterfactual.** Mixed tenses. The
present-tense half holds; the past-tense half did not, because
`CHANGELOG.md` got its copy of the marker from `534eac138` — the restore
commit itself — so during the absence a repo-wide grep for the evals
marker still matched nothing.
4. **"A four-row corpus" / "all five markers."** Counts that go stale as
rows are added, neither load-bearing. Now "per marker row" and "every
marker".

## Part 2 — two loose markers split into definition and use rows (#2915)

`--verify-restoration` tests a marker with `grep -qF`, which answers "is
this string anywhere in this file". Two markers were bare identifiers
occurring **twice** in their bound file:

| marker | definition | use site |
|---|---|---|
| `_FIND_SIDE_EFFECT_PRIMARIES` | `destructive_guard.py:1010` | `:1047`
|
| `MERGED_PR_GRAPHQL_ALIAS_PAGE` | `audit-fleet.sh:286` | `:2257` |

A re-land restoring only the use site would satisfy the grep and the row
would report `ok` — a partial re-land reported as restored, exactly the
failure the assertion exists to catch (#2828 was a partial re-land
nobody noticed).

The first pass narrowed each marker to its definition line. Review
pointed out that this closes only one direction: a re-land restoring the
definition while omitting the use would still report `ok`, leaving the
guard defined but disconnected. Narrowing cannot fix that — a single row
can only bind one of the two halves.

So each constant is now recorded as **two rows, a definition row and a
use row**:

```
marker 9239f15… destructive_guard.py _FIND_SIDE_EFFECT_PRIMARIES = frozenset(
marker 9239f15… destructive_guard.py name.casefold() for name in _FIND_SIDE_EFFECT_PRIMARIES
marker cc58cbc… audit-fleet.sh       MERGED_PR_GRAPHQL_ALIAS_PAGE=100
marker cc58cbc… audit-fleet.sh       gql_page_end=$((gql_page_start + MERGED_PR_GRAPHQL_ALIAS_PAGE))
```

A definition-only re-land passes the definition row and fails the use
row; a use-only re-land does the reverse. Each literal is the whole
expression rather than a bare identifier, and none depends on leading
whitespace — `parse_incidents_file` splits with `read -r kind sha path
rest`, so leading and trailing whitespace on the literal is stripped and
interior whitespace preserved.

**Being precise about what was broken:** both bare forms *did* still
discriminate the recorded incidents, because those reverts removed
definition and use together. Measured at the recorded revs, each bare
marker is absent at the reverting commit and present at its parent. The
weakness was never in what they assert about the past — it was that the
future shapes the assertion exists to catch could slip past them.

## Measurements behind this PR

All by `grep -oF … | wc -l` against the bound file, `git show
<rev>:<path>`, and `git show <rev>^:<path>`. Every marker row now in the
corpus, not only the changed ones:

| literal | bound file | worktree | `origin/main` | at parent | at rev |
|---|---|---|---|---|---|
| `Safe tidiness is the primary objective` | `disk-hygiene/README.md` |
1 | 1 | 1 | 0 |
| `empty-directories-remain-first-class-tidiness-findings` |
`clean/evals/evals.json` | 1 | 1 | 1 | 0 |
| `_FIND_SIDE_EFFECT_PRIMARIES = frozenset(` | `destructive_guard.py` |
1 | 1 | 1 | 0 |
| `name.casefold() for name in _FIND_SIDE_EFFECT_PRIMARIES` |
`destructive_guard.py` | 1 | 1 | 1 | 0 |
| `--apply-plan may be supplied only once` | `audit-fleet.sh` | 1 | 1 |
1 | 0 |
| `MERGED_PR_GRAPHQL_ALIAS_PAGE=100` | `audit-fleet.sh` | 1 | 1 | 1 | 0
|
| `gql_page_end=$((gql_page_start + MERGED_PR_GRAPHQL_ALIAS_PAGE))` |
`audit-fleet.sh` | 1 | 1 | 1 | 0 |

Every literal occurs exactly once in the file it binds to, is present at
the parent of its reverting commit, and is absent at the reverting
commit — the discrimination a marker must have. Both new literals were
additionally swept commit-by-commit and are present at every
first-parent commit that has touched their file since the restore.

## One attribution corrected

The corpus prose above the `9239f1541` row credited **#2640** with
restoring `destructive_guard.py`. Measured: #2640 (`6f0a31109`) touches
four files, all under `plugins/repo-fleet-hygiene`, none under
`plugins/disk-hygiene`. Both markers are absent at the two intervening
first-parent commits that touched the file (`224b04070` #2671,
`1a612904b` #2676) and first appear at `de26c27b4` (#2706). The note now
credits #2706 and records the correction in the file's own established
shape. The #2640 credits on the `cc58cbc53` row are correct —
`6f0a31109` *is* the `audit-fleet.sh` restore — and were left alone.

## Verification note

The comment changes touch no executable behavior. The marker rows are
data the replay reads, so `--verify-restoration` is the check that
matters and it runs in CI on this PR. The suite was **not** run locally
against this tree — CI is the verification of record.

Closes #2917
Closes #2915

## Related

- #2880 — `attribute_file` swallows blame stderr, so counts are lower
bounds
- #2879 — `clean` rows carry no attribution field, so nothing in CI
asserts their figures

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

disk-hygiene: the PowerShell belt does not flag >> append redirection

1 participant