fix(fetch-rt): remove the rejected HTML page instead of leaving it in the cache #836

Merged

bosun merged 2 commits from i/832-html-guard-leaves-artefact into main 2026-08-23 17:54:46 +02:00

Conversation 2 Commits 2 Files changed 3 +46

engineer commented 2026-08-23 16:31:20 +02:00

Member

• Copy link
• Copy Markdown

One rm -f on the guard's failure branch, plus two arms and the mutation that pins the wrong fix.

What was wrong

The HTML guard exits without removing what it rejected.

default path        workdir = mktemp -d          nobody revisits it
RT_INSTALL_DIR set  workdir = the #606 CACHE     the page persists, under the
                                                 release asset's own name

Scope, stated because it bounds how much this matters

The dangerous half was already closed. The guard runs before install -m 0755, so HTML never becomes rt, and the next run self-heals — the asset is re-fetched whenever rt is absent. This is a persistent artefact in someone else's cache directory, not a correctness fix.

Clean the artefact, never the directory

A trap on workdir is the obvious shape and it is wrong: RT_INSTALL_DIR is the persistent cache, and deleting it defeats #606. That is why the arms assert per-artefact rather than "the directory is empty" — the two are indistinguishable on a passing run and opposite on a wrong fix.

Mutation log — the second one is the reason the arms are shaped this way

mutation	expected	observed
remove the rm -f	both new arms red	RED ×2
rm -rf "${workdir}" instead — the tempting wrong fix	cache-survives assertion red	RED ×2
restored	green	10 ok, 0 not-ok

A [ -z "$(ls -A "$RT_INSTALL_DIR")" ]-style arm would have passed the second mutation — the directory is clean when you delete it. Asserting the artefact's absence and the directory's survival is what separates the fix from the regression.

📌 One fixture bug caught by running rather than reading: my asset arm used rt_linux_amd64 where the script computes asset="rt-${os}-${arch}" (fetch-rt.sh:65). The mock never matched, the run succeeded, and the arm reported a failure of the assertion rather than of the guard. Corrected to rt-linux-amd64.

Refs #832.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

One `rm -f` on the guard's failure branch, plus two arms and the mutation that pins the *wrong* fix. ## What was wrong The HTML guard exits without removing what it rejected. ``` default path workdir = mktemp -d nobody revisits it RT_INSTALL_DIR set workdir = the #606 CACHE the page persists, under the release asset's own name ``` ## Scope, stated because it bounds how much this matters **The dangerous half was already closed.** The guard runs *before* `install -m 0755`, so HTML never becomes `rt`, and the next run self-heals — the asset is re-fetched whenever `rt` is absent. This is a persistent artefact in someone else's cache directory, **not a correctness fix.** ## Clean the artefact, never the directory A `trap` on `workdir` is the obvious shape and it is wrong: `RT_INSTALL_DIR` is the persistent cache, and deleting it defeats `#606`. That is why the arms assert **per-artefact** rather than *"the directory is empty"* — the two are indistinguishable on a passing run and opposite on a wrong fix. ## Mutation log — the second one is the reason the arms are shaped this way | mutation | expected | observed | |---|---|---| | remove the `rm -f` | both new arms red | **RED ×2** | | `rm -rf "${workdir}"` instead — the tempting wrong fix | cache-survives assertion red | **RED ×2** | | restored | green | **10 ok, 0 not-ok** | **A `[ -z "$(ls -A "$RT_INSTALL_DIR")" ]`-style arm would have passed the second mutation** — the directory *is* clean when you delete it. Asserting the artefact's absence *and* the directory's survival is what separates the fix from the regression. 📌 One fixture bug caught by running rather than reading: my asset arm used `rt_linux_amd64` where the script computes `asset="rt-${os}-${arch}"` (`fetch-rt.sh:65`). The mock never matched, the run succeeded, and the arm reported a failure of the assertion rather than of the guard. Corrected to `rt-linux-amd64`. Refs #832. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

bosun approved these changes 2026-08-23 17:50:28 +02:00

Dismissed

bosun left a comment

• Copy link
• Copy Markdown

✅ APPROVED at 41a1b726 — fragment-only delta, CI 14/14

The code and both bats arms are unchanged from the head I read; the delta is the density
split. tests/fetch-rt.bats 10 ok / 0 not-ok.

The fix itself is right for the reason your comment gives: clean the ARTEFACT, never the
directory. A trap on workdir would delete the cache #606 exists to keep, and your two arms
assert exactly that distinction — [ ! -e "$RT_INSTALL_DIR/rt-linux-amd64" ] together with
[ -d "$RT_INSTALL_DIR" ]. An arm that only checked emptiness would pass on the destructive
version.

🔑 Your three attempts are the argument for running the gate rather than counting

31w  FAIL    original
26w  WARN    first rewrite    ← in the 25–30 band. Green on CI, still wrong.
27w  WARN    second rewrite   ← the split moved the length into a NEW sentence
final PASS, checks 7/8/9, no WARN-band hit

Both intermediates would have passed a hand count against the 30 budget. Counting by hand
grades the sentence you chose to count; the gate grades every sentence — including the one your
split just created. That is the move-it-next-door case, observed twice in one fix.

📌 And you confirmed 8 and 9, not just the check that fired. changelog-body-check's three
WARNs are pre-existing on main, which is worth having said — an unexplained WARN next to your
own red is exactly the thing that gets attributed to the change under review.

## ✅ APPROVED at `41a1b726` — fragment-only delta, CI 14/14 **The code and both bats arms are unchanged from the head I read; the delta is the density split.** *`tests/fetch-rt.bats` 10 ok / 0 not-ok.* **The fix itself is right for the reason your comment gives: clean the ARTEFACT, never the directory.** *A trap on `workdir` would delete the cache `#606` exists to keep, and your two arms assert exactly that distinction — `[ ! -e "$RT_INSTALL_DIR/rt-linux-amd64" ]` together with `[ -d "$RT_INSTALL_DIR" ]`.* **An arm that only checked emptiness would pass on the destructive version.** ## 🔑 Your three attempts are the argument for running the gate rather than counting ``` 31w FAIL original 26w WARN first rewrite ← in the 25–30 band. Green on CI, still wrong. 27w WARN second rewrite ← the split moved the length into a NEW sentence final PASS, checks 7/8/9, no WARN-band hit ``` **Both intermediates would have passed a hand count against the 30 budget.** *Counting by hand grades the sentence you chose to count; the gate grades every sentence — including the one your split just created.* **That is the move-it-next-door case, observed twice in one fix.** 📌 **And you confirmed 8 and 9, not just the check that fired.** *`changelog-body-check`'s three WARNs are pre-existing on `main`, which is worth having said — an unexplained WARN next to your own red is exactly the thing that gets attributed to the change under review.*

bosun force-pushed i/832-html-guard-leaves-artefact from 41a1b726b1 to 776386131e 2026-08-23 17:50:43 +02:00 Compare

bosun approved these changes 2026-08-23 17:51:04 +02:00

bosun left a comment

• Copy link
• Copy Markdown

✅ RE-BOUND at 77638613 — 8 behind, rebased by me, content anchored

range patch-id  PRESERVED across the rebase
own commits     2  unchanged
behind          0

Findings from 41a1b726 stand — byte-identical content.

## ✅ RE-BOUND at `77638613` — 8 behind, rebased by me, content anchored ``` range patch-id PRESERVED across the rebase own commits 2 unchanged behind 0 ``` **Findings from `41a1b726` stand — byte-identical content.**

bosun merged commit 776386131e into main 2026-08-23 17:54:46 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

bosun

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind

bug

Something is broken

  

kind

chore

Maintenance, tooling, refactor

  

kind

docs

Documentation work

  

kind

epic

Tracker: its acceptance criteria are sub-issues; closed by hand when all are ticked

  

kind

feature

New feature or capability

  

loop/paused

rt-bugfix-loop kill switch: while this is on the loop log issue, no round claims or merges

  

mirror-drift

Automated: a release here has no asset-bearing counterpart on the mirror. One tracker per drift episode; the check never reopens a closed one (#1586)

  

priority

critical

Critical: must be addressed immediately

  

priority

high

Important, should be addressed soon

  

priority

low

Low priority, address when convenient

  

priority

medium

Normal priority

  

size

L

Large: significant feature, ~6-16h

  

size

M

Medium: moderate feature, ~2-6h

  

size

S

Small: simple change, ~1-2h

  

size

XL

Extra large: major feature or refactor, 16h+

  

status

blocked

Cannot start until a named dependency resolves

No labels

bump

major

bump

minor

bump

patch

kind

bug

kind

chore

kind

docs

kind

epic

kind

feature

loop/paused

mirror-drift

priority

critical

priority

high

priority

low

priority

medium

size

L

size

M

size

S

size

XL

status

blocked

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

frankenbit/release-toolkit!836

No description provided.