Skip to content

Refuse a repeated undo of a withdrawn reaction instead of faulting on it - #104

Merged
max-sixty merged 1 commit into
mainfrom
fix/undo-withdrawn-reaction
Aug 29, 2026
Merged

Refuse a repeated undo of a withdrawn reaction instead of faulting on it#104
max-sixty merged 1 commit into
mainfrom
fix/undo-withdrawn-reaction

Conversation

@leaf-agent

Copy link
Copy Markdown
Collaborator

undo_error raised StopIteration out of POST /api/event when the undo named a reaction that had already been taken back. The withdrawn check sat below the per-kind ones, and the reaction branch walks build_threads for the thread holding the target — a fold that drops withdrawn events, so the walk had nothing to find and next() had no default. The fix moves the withdrawn check above the kind dispatch: whether a gesture still stands is a fact about the target, not about its kind, and every question below it reasons about a gesture that does.

This is the racing-tabs case the function's own docstring names — "the second is a no-op, and refusing it costs a toast". For a resolve or an action it does cost a toast. For a reaction it cost a 500 {"error": "StopIteration: "}, and _answer in http.py deliberately marks an unexpected fault retryable rather than final — so the browser retried the same attempt against a state that will never answer differently, wedging the gesture in the outbox instead of putting it back.

The other caller is unaffected: _browser_undo_candidates skips event["id"] in withdrawn before asking, so it never reached the raise and never reaches the new early return.

Verification

The regression test extends test_the_door_admits_a_reaction_only_as_a_token_the_layer_declares, which already presses undo on a reply reaction once and expects 200. It now presses a second time and expects the final 400. Against the unpatched events.py it reproduces the fault through the real HTTP door:

>       assert status == 400, body
E       AssertionError: b'{"error": "StopIteration: "}'
E       assert 500 == 400

Every other refusal message is unchanged — a spoken comment is still "is not a reaction", an undo is still "undo events cannot be taken back", and a repeated undo of a resolve still reads "already been taken back". None of the kinds routed around the moved check can be in taken_back in the first place: the door refuses their withdrawal, so nothing can have withdrawn them.

Ran locally: tests/test_interact_contract.py (217 passed), plus test_interact_server.py, test_interact_document.py, test_interact_session.py, test_interact_product.py (313 passed). The browser suites did not run — this sandbox has no Playwright browsers installed — so CI is their first pass. The change is Python-only and the browser-facing caller pre-filters, so no browser behaviour moves.

@max-sixty
max-sixty merged commit 82cfda2 into main Aug 29, 2026
4 checks passed
@max-sixty
max-sixty deleted the fix/undo-withdrawn-reaction branch August 29, 2026 21:40
max-sixty pushed a commit that referenced this pull request Aug 29, 2026
…n the suite (#106)

Tend sessions start with an empty Playwright cache, so the suite that is
most of `tests/` cannot run from one — `setup:` installs browsers under
the runner's home, which the sandbox does not inherit. This adds a
`sandbox_setup:` block that seeds the same headless shell `ci.yaml` uses
into the sandbox's own home, and asserts it by launching it. Verified in
a real tend sandbox: cache empty at start, install 4.4s, launch 0.8s,
and `uv run pytest --run-nightly -n0 tests/test_render_reactions.py -k
stacks_at_a_narrow_edge` then passes 2 tests in 4.26s where before it
could not start.

<details><summary>What was observed, and why this shape</summary>

### The evidence

Last night's `tend-nightly` session
([33238853704](https://github.com/max-sixty/leaf/actions/runs/33238853704))
reached this partway through the suite:

```
ERROR tests/test_render_conversations.py::test_the_line_offers_the_list_its_own_keys_rather_than_the_way_deeper_in
  playwright._impl._errors.Error: BrowserType.launch: Executable doesn't exist at
  /home/tend-sandbox/.cache/ms-playwright/chromium_headless_shell-1234/chrome-headless-shell-linux64/chrome-headless-shell
```

It did not install the browser; it stopped verifying locally and said so
in its summary — "The browser suites also did not run locally — this
sandbox has no Playwright browsers installed — so CI was their first
pass on both PRs". Both PRs it opened (#104, #105) are green, so nothing
wrong went out. What went out unverified is the point: on a repo where
nearly every test drives a real browser, the session shipped with the
repo's own gate unrun.

Three `tend-ci-fix` sessions the same night did install it themselves,
and two of them lost a turn to the same wrong flag first:

| Run | Command | Result |
| --- | --- | --- |
|
[33229417627](https://github.com/max-sixty/leaf/actions/runs/33229417627)
| `playwright install --with-deps chromium --only-shell` | `sudo: a
password is required` |
|
[33229417627](https://github.com/max-sixty/leaf/actions/runs/33229417627)
| `playwright install chromium --only-shell` | ok, 3s |
|
[33230715465](https://github.com/max-sixty/leaf/actions/runs/33230715465)
| `playwright install --with-deps chromium --only-shell` | `sudo: a
password is required` |
|
[33230715465](https://github.com/max-sixty/leaf/actions/runs/33230715465)
| `playwright install chromium --only-shell` | ok, 4s |
|
[33231657192](https://github.com/max-sixty/leaf/actions/runs/33231657192)
| `playwright install chromium --only-shell` | ok, 5s |

`--with-deps` switches to root, and the sandbox user has no sudo — the
trap `running-in-ci`'s Privileges bullet names, and its prescribed
remedy is exactly this: propose the setup entry rather than work around
its absence.

### Why `sandbox_setup:` rather than `setup:`

`setup:` runs as the runner. Playwright downloads to
`$HOME/.cache/ms-playwright`, and per tend's config reference the
sandbox inherits shared system and toolcache paths but never the
runner's home. `sandbox_setup:` runs as the sandbox user with the
workspace as cwd, which puts the browser where `sync_playwright()` will
look for it. This is the neighbour of the `uv`-on-PATH gap that
[max-sixty/tend#1071](max-sixty/tend#1071) fixed
upstream for 0.1.20 — same boundary, but a home-scoped download rather
than a PATH entry, so the repo owns it rather than tend.

### Why no `--with-deps`

The runner image already carries the system libraries: the three
sessions above ran the browser suite successfully after a plain
`--only-shell` install, and so did the verification here. Adding a
second `setup:` step to run `--with-deps` as the runner would be
machinery for a need that has not appeared.

### Cost

~5.3s per session that boots an agent (4.4s install, 0.8s launch,
measured in this sandbox). `sandbox_setup` runs inside the
`max-sixty/tend/claude` step, so the `tend-notifications` and
`tend-review` runs that short-circuit before booting pay nothing — and
`tend-ci-fix`'s gate-skips never reach it either.

### The regenerated workflows

`.config/tend.yaml` is the source; `sandbox_setup` is an action input,
so it has to reach each generated workflow. Regenerated with `uvx
tend@0.1.20 init` — the version already pinned in every workflow header
— so the diff is three added lines per file and no other drift, which
also confirms the committed files match what 0.1.20 generates.

### Gate assessment

- **Confidence**: High. Structural — the sandbox home starts without the
cache on every run, so it recurs every time. Four sessions hit it in one
20-hour window: one abandoned the gate, three installed it themselves,
two of those after a failed escalation.
- **Magnitude**: targeted fix. One existing knob in one place, plus the
mechanical regeneration it requires.
- **Cost**: mostly wasted compute — a turn per session and a download
per session. The nightly case is the one that isn't purely waste: a repo
whose safety net is a browser suite had that net unavailable to the
session doing the surveying. No wrong outward action occurred, so the
remedy is held to the waste-class bar, which one config knob meets.

</details>

Co-authored-by: leaf-agent <318509791+leaf-agent@users.noreply.github.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.

2 participants