Repository navigation
skills(running-tend): two new triage rules from PR #2633 feedback - #2639
Merged
Merged
Conversation
PR #2633 was a bad triage outcome: tend opened a parallel narrow PR for a Nix-flake nightly failure with a test-skip "fix" while PR #2625 was already in flight fixing the same root-cause class via production code. Maintainer feedback in #2633 ([comment][c]): > this was bad: it should have avoided creating a PR assuming the > outstanding PR would have fix the issue / and the change was ti > change the tests! but we should have fixed the code! / consider > what changes we could make to tend Two new H3 sections under `## Issue Triage`: 1. **Don't fix tests by adding skip guards.** Smell list of patterns (`let Ok(_) = ... else { return };`, `if !path.exists() { return; }`, newly-added `#[ignore]`) that should make a triage attempt stop and ask whether the production code or test setup is the real fix site. 2. **Same-root-cause-class triage.** The existing "work on the outstanding PR instead" rule keys on same-test overlap; this adds the multi-test-same-root-cause case where the right move is to wait for the in-flight PR and re-run, or mirror its approach. Both anchored to PR #2633 / PR #2625 so future triage runs can see the incident the rule was distilled from. [c]: #2633 (comment) Co-Authored-By: Claude <noreply@anthropic.com>
4 tasks done
Owner
|
ok but reduce verbosity and don't include past cases (and if they exist elsewhere in this file then remove them) |
Collaborator
Author
|
Tightened both new sections and dropped the |
max-sixty
pushed a commit
to max-sixty/tend
that referenced
this pull request
May 8, 2026
…oot-cause PRs (#394) ## Problem Triage feedback from a run in `max-sixty/worktrunk` (PR [#2633](max-sixty/worktrunk#2633)) surfaced two patterns the bundled `triage` skill doesn't currently guard against: 1. The triage run "fixed" a failing test by inserting an early-return that silently bailed when the bug fired, removing coverage instead of restoring the assertion. 2. An outstanding PR ([worktrunk #2625](max-sixty/worktrunk#2625)) had been merged ~3 minutes earlier with a production-code fix for the same root-cause class. The triage PR's own body even named it as same-root-cause — but Step 3's duplicate check only catches identical fixes, not the *same root cause class, different surface* pattern, so the run shipped a parallel narrow workaround anyway. worktrunk's overlay captured both rules in [worktrunk PR #2639](max-sixty/worktrunk#2639); both look generalizable to every tend consumer, hence promoting them to the bundled skill. ## Solution Two new subsections under Step 6 of `plugins/tend-ci-runner/skills/triage/SKILL.md`: - **Don't "fix" tests by adding skip guards** — names the smell patterns (newly-added early-returns, fresh `#[ignore]`, `pytest.skip` keyed on the failing condition). - **Defer to in-flight same-root-cause PRs** — when triage analysis itself names an existing PR as same-root-cause, that's the signal to wait, not to ship a parallel workaround. Both are strict additions. Step 3's existing duplicate-check guidance and Step 6's "Fix the root cause (not just the symptom)" remain in place; the new rules fill gaps neither covered. ## Testing `pre-commit run --files plugins/tend-ci-runner/skills/triage/SKILL.md` passes (trim trailing whitespace, fix end of files, typos, forbid bang-backtick). --- Closes #393 — automated triage Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
max-sixty
pushed a commit
that referenced
this pull request
Sep 19, 2026
…l now carries (#4189) Two H3 sections under `## Issue Triage` in the `running-tend` overlay restate rules that tend's bundled `fix-a-bug` skill carries, so the overlay copies can only drift from them — and one already has. - **`Don't fix tests by adding skip guards`** duplicates bundled **Don't "fix" tests by adding skip guards**, down to the same smell list (`let Ok(_) = … else { return };`, `if !path.exists() { return; }`, `#[ignore]`). The bundled version is already wider: it adds `skipIf` / `pytest.skip`, and it leads with the test that generalizes — does the change restore the assertion, or remove coverage for the failing scenario? - **`Same-root-cause-class triage`** duplicates bundled **Defer to in-flight same-root-cause PRs**, with nothing left over. The drift that matters is scope. Both overlay copies sat under `## Issue Triage`, so they read as triage-only rules; `fix-a-bug` is loaded before any fix in any workflow, so the bundled rules bind a nightly sweep and a CI-fix run too. A session reading the overlay alone could conclude a skip guard is fair game outside triage. That's the same correction #4086 made for the root-cause rule. So this PR deletes both sections and keeps only what is genuinely worktrunk's, as an addition to the bundled rule rather than a restatement of it: a test leaning on inherited state reworks its own setup via `TestRepo::with_initial_commit()` plus a tempdir. That survivor moves to a top-level section beside the test commands, since it is no more triage-specific than the rules it defers to, and says so explicitly. Both rules were added by #2639, whose own body flagged that the skip-guard one "may also generalize beyond worktrunk" — it since has. No regression test: this is instruction text with no code path to exercise. The overlay carries no cross-references to either heading, and `.agents/skills` is a symlink to `.claude/skills`, so the Codex view follows automatically. --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #2633 was a bad triage outcome — closed by the maintainer with feedback asking what changes we could make to tend:
Two distinct mistakes from that run, each addressed by a new H3 under
## Issue Triageinrunning-tend:let Ok(_) = Repository::current() else { return };so the test would silently bail in the Nix flake build sandbox. That removes coverage instead of fixing the real issue. The new section gives a smell list (early-return skips, newly-added#[ignore],if !path.exists() { return; }) and tells the run to stop and look at production code or test setup instead.Both rules anchored to PR #2633 / PR #2625 / issue #2632 so future runs can see the incident they were distilled from.
The "don't fix tests by adding skip guards" rule may also generalize beyond worktrunk — happy to file a follow-up issue against the bundled
triageskill if you think it belongs there too.Test plan
## Issue Triage