fix: contain termimad panic on ragged tables in PR comments

[worktrunk-bot]

Problem

Running wt switch (the interactive picker) crashes with:

thread '<unnamed>' panicked at termimad-0.34.1/src/tbl.rs:176:30:
index out of bounds: the len is 7 but the index is 7
Rayon: detected unexpected panic; aborting

The panic originates in the termimad crate (latest version, 0.34.1 — no upstream fix available), reached via the picker's PR-comments preview pane: picker::prs::render_comment_blocks → md_help::render_table_with_termimad → MadSkin::text.

Root cause (upstream bug): termimad's column fitter, Table::fix_columns, panics on a ragged table — one where some row has more cells than the header — when it's rendered at a width too narrow to fit the widest row. In that case the fitter's TblFit::new fails and it takes an error branch that skips padding the short rows to the column count, then the alignment loop indexes past them (cells[ic] for ic in 0..nbcols). Equal-column tables never hit this at any width; only ragged ones do.

PR/MR comment markdown is untrusted and can contain such a table. Because the comments pane renders on a rayon worker, the escaped panic aborts the entire process rather than just failing one render.

Solution

Contain the upstream panic at the single termimad call site (render_table_with_termimad in src/md_help.rs) with std::panic::catch_unwind, falling back to the plain preprocessed table lines so the pane still shows the table's text. This keeps the panic from escaping to the rayon worker and taking down wt switch, and guards the whole class of termimad table panics rather than one specific input.

Testing

Added test_render_table_ragged_narrow_does_not_panic in src/md_help.rs, which renders a ragged table (| Key | Value | header over a six-column data row) at a narrow width. Verified it panics at tbl.rs:176:30 without the fix (same location as the report) and passes with it. Full md_help suite (26 tests) and cargo clippy are green.

---

Closes #3407 — automated triage

[worktrunk-bot]

Self-review of the termimad panic containment. The core fix is sound — catch_unwind around the single termimad call is a clean, minimal way to stop the rayon-worker abort from taking down wt switch, AssertUnwindSafe is justified (the closure only reads skin/markdown/width and nothing inconsistent is observed afterward), and there's no panic = "abort" profile so the unwind is actually catchable. One design observation worth a human's eye before merge, plus a smaller test note.

The panic message still reaches stderr and can smear the picker frame. catch_unwind swallows the unwind, but Rust's default panic hook runs first and writes thread panicked at termimad-0.34.1/src/tbl.rs:176:30 ... to stderr. This module (src/commands/picker/mod.rs) deliberately stashes its own eprintln output and drains it to stderr only "once the picker releases the terminal" (see the doc comments around mod.rs lines ~146 and ~1162) precisely because writing to stderr while skim owns the terminal corrupts the display. So on the rare trigger this converts a hard abort into a one-time garbled frame (recovers on the next repaint) rather than a fully clean fallback — still a clear improvement, but not the silent degrade the comment implies. A truly clean fallback would need to suppress that hook write (a process-wide panic::set_hook installed at startup, since a scoped set_hook is racy across concurrent preview workers). Reasonable to defer, but worth a conscious call rather than leaving it implicit.

The regression test doesn't distinguish the fallback from a successful render. test_render_table_ragged_narrow_does_not_panic asserts only that plain contains "alpha" and "zeta" — but a successful termimad render of the same table also contains those cell texts, so the assertion passes on either path. It currently exercises the fallback (you verified the panic fires without the fix), but if a future termimad release fixes the ragged-table bug or the width-20 reproducer stops tripping the fitter arithmetic, the test stays green while silently no longer covering catch_unwind. Asserting on something only the fallback produces (e.g. the raw | pipe delimiters from processed.join, which a real termimad render replaces with box-drawing borders) would keep it honest.

Not flagging the shared-leaf vs. rayon-boundary placement: render_data_table builds equal-column tables by construction, so the trusted --help/config state callers can't be ragged and won't hit this path — the broadened containment is practically harmless.

[max-sixty]

also please draft an issue for me to put into termiad; check whether one still exists

[worktrunk-bot]

Upstream termimad issue

Existing issue check: I searched Canop/termimad (open + closed) for panic, index out of bounds, table, tbl, fix_columns, ragged — no existing issue covers this. The three closed panic reports (#7, #22, #23) are all broken-pipe panics in print_text, unrelated to the table fitter. So this is new — draft below, ready to paste.

I confirmed the reproducer against a standalone termimad = "=0.34.1" binary (no worktrunk code): it panics at exactly tbl.rs:176:30 — same location as the crash report. I also mapped the width band: the ragged table panics at widths 16/20/24, renders fine at 8/12 and at 30+, and the equal-column version never panics at any width. That's all folded into the draft.

Draft issue for Canop/termimad (raw markdown, copy-paste)

### Panic: index out of bounds in `Table::fix_columns` on a ragged table at narrow width

`MadSkin::text` (and any render path through `Table::fix_columns`) panics with an out-of-bounds index when a **ragged** table — one where a row has more cells than the header — is rendered at a width too narrow to fit the widest row.

#### Reproducer

```rust
// termimad = "=0.34.1"
use termimad::MadSkin;

fn main() {
    let skin = MadSkin::default();
    // Header has 2 columns; the data row has 6.
    let md = "| Key | Value |\n\
              | --- | --- |\n\
              | alpha | beta | gamma | delta | epsilon | zeta |\n";
    // Panics at tbl.rs:176:30
    let _ = skin.text(md, Some(20)).to_string();
}
```

#### Observed

```
thread 'main' panicked at termimad-0.34.1/src/tbl.rs:176:30:
index out of bounds: the len is 2 but the index is 2
```

#### When it happens

Only for **ragged** tables, and only in a band of widths that are too narrow to fit the widest row but not tiny:

| table | width | result |
|-------|-------|--------|
| ragged (2-col header, 6-col row) | 8, 12 | ok |
| ragged | 16, 20, 24 | **panic** |
| ragged | 30, 40, 60, 200 | ok |
| equal-column (2-col header, 2-col row) | 4, 20, 40 | ok |

#### Mechanism

In [`Table::fix_columns`](https://github.com/Canop/termimad/blob/bc20f734ee09a900960da6c90170fb6e7b5a861b/src/tbl.rs#L94), `nbcols` is the table's column count (the widest row for a ragged table). When [`TblFit::new(nbcols, width)` returns `Err`](https://github.com/Canop/termimad/blob/bc20f734ee09a900960da6c90170fb6e7b5a861b/src/tbl.rs#L113-L118) — the "not enough width, remove columns" path — `nbcols` is recomputed to `(width - 1) / 4`, but that branch **skips** the per-row cell-padding loop that the `Ok` branch runs. Padding is then left to the resize loop, where a cell wider than its column is wrapped and its overflow inserted as a new continuation row; that inserted row is built from `cells_to_add`, which only has entries for the row's original cells, so it can end up with fewer than `nbcols` cells. The final alignment loop then does [`cells[ic]` for `ic in 0..nbcols`](https://github.com/Canop/termimad/blob/bc20f734ee09a900960da6c90170fb6e7b5a861b/src/tbl.rs#L175-L176) on every row and indexes past the short one — the panic.

#### Suggested fix

Guarantee every row (including inserted continuation rows) has exactly `nbcols` cells before the alignment loop — e.g. pad short rows in the `Err`/`cols_removed` branch the same way the `Ok` branch does, or bound-check the alignment loop with `cells.get(ic)`.

#### Environment

- termimad 0.34.1
- Reproduces via `MadSkin::text(markdown, Some(width))`; the input is untrusted markdown (a table pasted into a PR comment), so it's reachable from user data.

---

Two notes on the self-review, for the record:

• Test point (addressed): I pushed 1dbaf80 strengthening test_render_table_ragged_narrow_does_not_panic. It now also asserts the output contains a literal |, which only the plain-text fallback preserves — a successful termimad render replaces the delimiters with box-drawing │. The old cell-text-only assertion passed on either path, so it would have gone green (and stopped covering catch_unwind) if termimad ever fixed the bug upstream.
• stderr-smear point (deferred): left as-is — Rust's default panic hook still writes the panic message to stderr before catch_unwind swallows the unwind, so on the rare trigger the picker frame can smear once and recover on the next repaint, rather than degrading fully silently. A clean fix needs a process-wide panic::set_hook at startup; flagging for your call rather than doing it here.

[max-sixty]

@worktrunk-bot does this still happen with termimad 0.35?

[worktrunk-bot]

Yes — still reproduces in termimad 0.35.0 (published earlier today). Same bug, unfixed upstream.

I ran the original reproducer against a clean termimad = "0.35.0" scratch crate:

width   8: ok
width  12: ok
width  16: PANIC
width  20: PANIC
width  24: PANIC
width  30: ok
width  40: ok
width  60: ok
width 200: ok

Identical width band (16/20/24) and the same panic site — Table::fix_columns:

thread 'main' panicked at termimad-0.35.0/src/tbl.rs:176:30:
index out of bounds: the len is 2 but the index is 2

So the catch_unwind guard added here is still warranted; there's no upstream fix to depend on yet. I'd hold off on bumping to 0.35.0 for this reason — it buys nothing here. (If you want the bump for its other changes I can open a separate PR; the strengthened test_render_table_ragged_narrow_does_not_panic would keep covering the guard either way.)

[worktrunk-bot]

Surfacing a cross-repo question here, since I can't act on it in-thread.

Over in Canop/termimad#77 — the upstream issue for this panic — @Canop asked whether I'd open the fix PR or he should. I'm not able to open PRs (or comment) in repos outside this one, so I can't respond there or push the fix upstream myself. Two ways forward:

• Hand off the patch (recommended). I traced the bug to a one-line invariant gap; it's ready for you or @Canop to apply directly — details below.
• Relax the cross-repo rule. If you'd like me to open upstream PRs like this going forward, I can propose a running-tend change permitting it for agent-adjacent dependencies. That's a broad policy loosening, though, so I'd rather you decide it explicitly than have me infer it.

Verified fix for termimad tbl.rs (source-traced against v0.35.0, not runtime-tested)

The panic is in Table::fix_columns. When a table is too narrow to fit its widest row, the fitter takes the Err "remove columns" branch and skips the per-row cell-padding loop the Ok branch runs. The resize loop compensates by padding short rows through the cells.len() <= ic continue branch — but that branch pushes to cells without a matching push to cells_to_add, unlike the fall-through one line below. So cells_to_add ends up with fewer than nbcols entries; the continuation row built from it gets fewer than nbcols cells; and the final alignment loop's cells[ic] then indexes past it → index out of bounds: the len is N but the index is N.

Minimal fix — restore the invariant the Ok branch already maintains (every row and every cells_to_add slot has nbcols entries) by pushing an empty slot in the continue branch:

for ic in 0..nbcols {
    if cells.len() <= ic {
        cells.push(FmtComposite::new());
        cells_to_add.push(Vec::new()); // keep cells_to_add aligned with nbcols
        continue;
    }
    cells_to_add.push(Vec::new());
    // ...

The continues are always at the tail (cells are padded in index order), so cells_to_add stays index-aligned with ic. Equivalent alternatives if defense-in-depth is preferred: build the continuation row over 0..nbcols using cells_to_add.get_mut(ic), or bound the alignment loop with cells.get_mut(ic).

Verification level: traced against 1a22ecd (v0.35.0) and matched to the exact panic site (tbl.rs:176) and message; I did not compile a patched build. The reproducer already in the issue confirms the panic empirically across the narrow-width band.

[Canop]

Ok @worktrunk-bot I'll check your fix and do the change in Termimad

[worktrunk-bot]

Thanks @Canop, much appreciated — and thanks for maintaining termimad. The reproducer in #77 covers the whole narrow-width band (16/20/24) plus the ok cases on either side, so it should make a decent regression test if useful.

No rush on our end — we've got the catch_unwind guard containing it locally, so we're unblocked. Once a fixed release is out I'll drop the workaround and bump. Happy to help verify a patched build against the repro if that's handy.

[Canop]

version 0.35.1 has been published and has the fix

[worktrunk-bot]

Thanks @Canop — fast turnaround, much appreciated. Verified 0.35.1 renders the reproducer clean across the whole narrow-width band (16/20/24 and beyond), so I've opened #3422 to bump to it and drop the catch_unwind workaround this PR added.

[max-sixty]

Over in Canop/termimad#77 — the upstream issue for this panic — @Canop asked whether I'd open the fix PR or he should. I'm not able to open PRs (or comment) in repos outside this one, so I can't respond there or push the fix upstream myself. Two ways forward:

@worktrunk-bot we should change this guidance (maybe @tend-agent this is on you?)

It's correct that you shouldn't spam issues or PRs outside your area of ownership. But if a maintainer of a different project (or the project's policy) says it's OK for you to contribute, and it's helpful for the project you maintain, then you should do this. Probably this is a change in tend.

[worktrunk-bot]

Agreed — the current rule is unconditionally "no PRs/comments in other repos", which is right for spam but wrong when an upstream maintainer explicitly invites the contribution and it helps worktrunk. You're right that it's a tend change (it's a bundled skill, so it'd fire for every consumer, not just here), so I've filed it there: max-sixty/tend#769.

It proposes a bounded carve-out to the Scope rule — permit an upstream PR/comment when both hold: (1) an explicit invitation (a target-repo maintainer asks in-thread, or the repo's contributing policy welcomes it — not just inferred), and (2) it serves worktrunk (e.g. upstreaming a dependency fix we're working around). The anti-spam default stays; only the invited case is unblocked. Concrete skill edits are in the issue for @tend-agent / your review.

(This one's moot in practice now — @Canop shipped the fix in termimad 0.35.1 and #3422 bumps + drops the workaround — so it's purely so the next invited upstream contribution isn't blocked.)