Skip to content

skills(running-tend): restore outage drain until tend#851 lands - #3781

Open
worktrunk-bot wants to merge 6 commits into
mainfrom
skills/restore-outage-drain-31289143471
Open

skills(running-tend): restore outage drain until tend#851 lands#3781
worktrunk-bot wants to merge 6 commits into
mainfrom
skills/restore-outage-drain-31289143471

Conversation

@worktrunk-bot

@worktrunk-bot worktrunk-bot commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Restores the ## Outage Recovery: Re-run Triggers Stranded by a Failed Session hunk that #3742 removed. The 33 restored lines are byte-identical to what was deleted; on top of them sits one added line under the heading marking the section as a hold on max-sixty/tend#851, so the removal trigger is discoverable from SKILL.md itself rather than only from this description (added in review).

Why now. #3742 removed the section on the premise that the guidance would live upstream instead: "the guidance now lives in tend's bundled review-runs skill, proposed in max-sixty/tend#851." That PR has not landed — open since 2026-08-05 with no reviews — so the guidance currently exists in neither place. I checked the bundled skills rather than assuming: ci-fix only creates a tend-outage issue, nightly only enriches one via enrich-tend-outage-issues.sh, and review-runs (the target of the upstream PR) has no outage text at all.

The window cost more than it was priced at. #3742's own body flagged the sequencing and put a bound on it: "Merging this before #851 lands leaves a window with no drain guidance in either place. The window costs one daily sweep at most, and the outage issue accumulates rather than losing data." Last night's outage falsified the second half. A Claude session-limit exhaustion ran 22:23Z–23:55Z on 2026-08-08 and stranded nine rows on #3780. Seven were tend-notifications on schedule, which self-recover — correctly out of scope. The eighth was run 31281595492, tend-review for #3779, attempt 1 failed 22:26:53Z. tend-review fires only on pull_request_target, so nothing would have re-fired it. The maintainer re-ran it by hand — attempt 2 started 00:01:31Z, approved 00:05:35Z, merged 00:09:49Z. Without that manual rerun #3779 merges unreviewed, which is data lost rather than accumulated.

The accumulation half didn't hold either: three tend-notifications runs completed after the quota reset (00:12Z, 00:33Z, 01:08Z) and none of them touched #3780, because nothing in either skill set tells a run to drain it. This session drained and closed it by hand.

Lifetime. This is a hold, not a position on where the guidance belongs — #3742's argument that nothing in the section is worktrunk-specific still reads correctly. When max-sixty/tend#851 lands, drop this section again and the bundled version takes over. That instruction is now in the file, not just here.

Evidence
  • Removal: gh pr view 3742 --json mergedAt2026-08-05T16:57:13Z; the diff is a pure deletion of the 33-line section, and git log -- .claude/skills/running-tend/SKILL.md shows no later commit touching the file, so the restored lines are an exact reverse-apply of that hunk — the only divergence is the hold marker added in review.
  • Upstream still open: gh pr view 851 -R max-sixty/tend --json state,mergedAt,reviewsOPEN, mergedAt: null, reviews: [].
  • No bundled coverage: grep -rniE "tend-outage|stranded|outage recovery" over plugins/tend-ci-runner/skills/ → hits only in ci-fix (issue creation), nightly (enrichment), and review-reviewers (a label-skip note).
  • Stranded review: gh api repos/max-sixty/worktrunk/actions/runs/31281595492run_attempt: 2, run_started_at: 2026-08-09T00:01:31Z; attempt 1 conclusion: failure at 22:26:53Z.
  • Post-reset runs that left Bot temporarily unavailable #3780 open: 31285688990, 31286424710, 31287672220 — all success.
  • A second stranded trigger, run 31282964053 (tend-mention on repository_dispatch), also won't self-recover, but re-running it now would post into a merged thread — the section's "confirm the work is still missing before re-running" rule is what says to leave it.

#3742 removed the outage-recovery section on the premise that tend#851 would
carry it upstream. That PR has not landed, so the drain guidance exists in
neither place, and last night's session-limit outage stranded a tend-review run
that only a manual maintainer rerun recovered. Pure revert of the removed hunk.

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Verified the mechanical claim rather than taking it on faith: extracting the removed lines from 60830520c and the added lines from 2bfcd498e and diffing them shows the 33 lines are byte-identical, restored at the same position (after Labels, before CI Fix). The premise checks out too — max-sixty/tend#851 is still OPEN with reviews: [], and a grep over the bundled tend-ci-runner skills finds outage text only in ci-fix (issue creation), nightly (enrichment), and review-reviewers (a label-skip note); review-runs has none, so the guidance is currently in neither place.

One gap: the restored section is verbatim, so its stated lifetime lives only in this PR description. A future sweep reading SKILL.md has no signal that this is a hold — once tend#851 lands, the overlay silently duplicates the bundled guidance and the two drift independently. The bundled skills already use an in-file marker for exactly this shape (<!-- TODO(anthropics/claude-code#37157): once the harness exempts .claude/skills/ ... -->). Inline suggestion adds a one-line pointer; that costs the "pure revert" property, which seems worth trading for the removal trigger being discoverable from the file itself.

Comment thread .claude/skills/running-tend/SKILL.md
… one

The section this branch restores predates the two-limit correction: it
names only `session limit` and asserts a 5-hour window. tend#851, which
this branch holds for, carries the corrected text. Bring the local copy
in line so the hold isn't a regression.
@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

Pushed b796c514a — the restored section was the version from before the two-limit correction.

#3742 did two things in sequence: it first fixed this section (added the weekly-limit message shape, replaced the baked-in 5-hour figure with "read the reset off the message", gated the re-run on a later clean run), and then, after the routing correction in #3742 (comment), deleted the section and moved it upstream. So the merged #3742 diff is a pure deletion, and reverting that deletion brings back the uncorrected text. max-sixty/tend#851 — the PR this branch holds for — carries the corrected wording; the local copy did not.

The consequence is a timing instruction that's wrong by 3× on the weekly flavor: a run reading "quota exhaustion across a 5-hour window" during a weekly exhaustion re-runs stranded triggers while the quota is still out, and each re-run appends another row to the issue it is draining. That's the exact failure mode the correction was written for — #3737 (2026-08-04) was a weekly exhaustion lasting 15h 10m, against #3715 (2026-08-03) at 8h 20m for the session flavor.

The commit is 3 lines: the second message shape in the jq output comment, the two-clock sentence replacing the 5-hour assertion, and one word in the paragraph above (session-limitquota) now that two shapes are listed.

Verification
$ git show 60830520c --stat            # #3742 as merged
 .claude/skills/running-tend/SKILL.md | 33 ---------------------------------

Pure deletion — no corrected text ever landed on main.

$ gh pr diff 851 -R max-sixty/tend | grep '^+' | grep -iE 'weekly|reset'
+# → You've hit your session limit · resets 8:30am (UTC)
+# → You've hit your weekly limit · resets 12am (UTC)
+A cluster of these is quota exhaustion, not a bug … read the reset off the
+message rather than assuming the shorter session window; a weekly exhaustion
+can strand most of a day.

Found while reviewing this window's outage (#3780, session limit 22:23Z–23:55Z on 2026-08-08) for the daily run review.

@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

Pushed 21adcb61b — a second, unrelated line to the same file. Opening a separate PR would conflict with this branch, and the skill's own dedup rule is to add to an open PR against the same skill rather than open a second, so it rides here; happy to split it out if you'd rather keep this branch to the outage drain.

What it adds. One bullet at the top of the Weekly Maintenance: Statusline Cache-Check triage list, naming the -vv diagnostic epilogue as the first thing to check.

Why. The recipe requires -vv, and -vv writes the diagnostic bundle after the render finishes. Assembling it re-runs three commands the render already made — git --version and git worktree list --porcelain in DiagnosticReport::collect (src/diagnostic.rs:223, :229) and gh --version behind the gist hint (is_gh_installed). All three go through Cmd, so they are traced into the same trace.jsonl the .cache report reads, and pair with the render's own calls as same-context duplicates. So the measurement method injects the finding it then reports.

Three consecutive weekly runs re-derived this from scratch, each spending turns on it and each concluding "not a production cache miss, nothing to file":

Weekly run Duplicates flagged Conclusion reached
30197306136 (07-26) gh --version, git worktree list --porcelain, git config --list -z attributed to the -vv bundle, but the isolation test was called inconclusive; fell back to the 28 ≤ 29 baseline
30742679188 (08-02) same three attributed to the -vv epilogue, via reading #3705
31305900412 (08-09) gh --version, git --version, git worktree list --porcelain attributed to the -vv epilogue, separated by thread id (tid 1 at t≈5.91s vs the render's tid 17)

#3705 (merged 08-02) removed the git config --list -z inflation; the other three remain, because they are the bundle doing its job rather than a trace defect.

That last run said so itself and held back only on this branch being open: "One thing I did not do: codify that -vv false-positive in the running-tend cache-check section. It will resurface identically every week, but there's an open PR (#3781) against the same SKILL.md." This closes that.

Gate assessment. High evidence at 3 occurrences; structural (the recipe mandates -vv and the epilogue is unconditional, so a replay produces it every time); targeted fix inside an existing section, no new section. The bullet says what to check and how to tell — thread id and a timestamp past the render's end — rather than listing this incident.

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The outage-restore half was checked in the previous review, and b796c514's two-limit correction matches the wording carried in max-sixty/tend#851 — so this is about the 21adcb61 bullet. I pulled the .cache output the two cited weekly runs actually produced rather than taking the PR comment's table at face value; two of the bullet's claims don't survive that.

git --version isn't one of the duplicates. Its only spawn sites are git_version() in src/diagnostic.rs (called from format_report, i.e. the epilogue) and wt config show — nothing on the statusline render path runs it, so it fires exactly once and CacheReport, which only buckets a (command, context) pair seen more than once, can't surface it. Run 31305900412's own .cache lists exactly two entries: gh --version (context (none), count 2) and git worktree list --porcelain (context ., count 2). That matters beyond tidiness — if git --version ever did show up twice it would be a real finding, and this bullet currently pre-labels it as noise.

"explained every duplicate the report flagged on the last three runs" overstates it. Run 30742679188 (08-02) flagged three, and the third — git config --list -z, context .git — was a genuine in-process miss, fixed by #3705, not an epilogue artifact. A bullet that says "check this first, it explains everything" invites the next run to dismiss the one that isn't.

The discriminator the bullet names isn't in the recipe's output. jq .cache yields same_context_duplicates[] with command / max_per_context / extra_calls / extra_us / contexts and nothing else — no tid, no per-call timestamps (CacheReport and DuplicateCommand in src/trace/profile.rs). Those fields are in the trace.jsonl records themselves ({"kind":"cmd_completed","ts":…,"tid":…,"cmd":…}, per the src/trace/parse.rs module docs), which is why the 08-09 run ended up grepping the file directly. Naming the file and the query is exactly the kind of re-derivation this bullet exists to stop.

Suggestions inline. Not approving — self-authored.

Comment thread .claude/skills/running-tend/SKILL.md Outdated
Comment thread .claude/skills/running-tend/SKILL.md Outdated
… the trace query

`git --version` has one spawn site on this path — `git_version()` in
`src/diagnostic.rs`, reached from `format_report` (the epilogue) and from
`wt config show` — so it runs once per invocation and can never form a
duplicate bucket. Verified on a real `-vv` statusline run: the `.cache`
report lists exactly `gh --version` and `git worktree list --porcelain`,
and `git --version` appears once in `trace.jsonl`.

Soften "every duplicate on the last three runs" to what the runs show —
the 08-02 run's third duplicate (`git config --list -z`) was a real miss
fixed by #3705, so a blanket dismissal invites the next run to wave through
the one that isn't an artifact.

Name the query that answers the artifact test: `jq .cache` carries no `tid`
or `ts` (`CacheReport`/`DuplicateCommand` in `src/trace/profile.rs`), so
the check has to run against `trace.jsonl`. Path goes through
`git rev-parse --git-common-dir` rather than a literal `.git/` — the recipe
says "run from any worktree", where `.git` is a file and the literal path
fails with ENOTDIR.

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Re-checked both findings from the last review against the code rather than the commit message. Both hold up:

  • git_version() really has only the two callers (format_report, wt config show), so it can't form a bucket — and same_context_duplicates keys on the raw command string from the trace, not command_type's shape (DuplicateCommand construction in src/trace/profile.rs), which is what makes the new select(.cmd == "gh --version") query match the .cache entry exactly rather than a gh bucket. Worth knowing since command_type("gh --version") collapses to gh.
  • The path fix is right and matches the idiom this skill already uses for diagnostic.md: wt_logs_dir() is wt_dir().join("logs") over git_common_dir(), so $(git rev-parse --git-common-dir)/wt/logs/trace.jsonl resolves from a linked worktree where the literal .git/ path is a file.

One anchor left over: the bullet cites DiagnosticReport::collect for the git worktree list --porcelain re-run, but that spawn is in format_reportcollect just calls it, so a grep for collect lands one hop short. The commit message for bab1e97b7 already names format_report; the bullet is what didn't get updated. Suggestion inline.

Not approving — self-authored.

Comment thread .claude/skills/running-tend/SKILL.md Outdated
`DiagnosticReport::collect` forwards to `format_report`, which is where the
`git worktree list --porcelain` call actually sits (src/diagnostic.rs:230),
so a grep for `collect` lands one hop short of the spawn.
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.

1 participant