chore(skill): prefer reusing existing code for repo homogeneity (#2952)

* chore(skill): prefer reusing existing code for repo homogeneity

The standing rule already demanded the simplest solution; it said nothing
about where that solution should come from. A bespoke-but-simple mechanism
in one add-on is still a second way to solve a problem 120+ add-ons share.

- Standing rule: build out of what exists (.templates/ module, existing
  cont-init script, a sibling add-on's pattern), and match repo naming
  conventions when something new is genuinely needed.
- Step 3 (Plan): search for prior art before ranking mechanism levels; not
  reusing an existing mechanism now requires stating why.
- Step 5 (Simplify): reuse check alongside the existing ones — fold
  near-duplicates in, or justify the divergence in the PR body.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(skill): address Codex and CodeRabbit review feedback

- Prior-art search: the --include='*.sh' --include='config.yaml' allowlist
  missed the repo's main mechanisms. `ARG MODULES=` lives in Dockerfiles and
  s6 v3 services are extensionless `run` files; searching for MODULES= found
  6 files under the allowlist vs 129 (125 Dockerfiles) without it. Widened to
  --exclude-dir=.git and named the two file types explicitly.
- Reuse vs isolation: "fold a near-duplicate into the existing mechanism"
  contradicted traps.md:125, which requires a new numbered script rather than
  editing scripts shared by symlink with the webtop add-ons. Added the carve-out.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

.claude/skills/hassio-addon-workflow/SKILL.md

@@ -25,9 +25,14 @@ Triage first, then one of two paths:
 Escalate mid-flight if a light task grows — touches a default, needs a new script or service, or
 reveals a deeper problem.
 
-**Standing rule:** ship the simplest solution that works. Complexity is bought only by a
-**measurement** showing a concrete, user-visible cost on a real host — never by reasoning about
-hypothetical performance.
+**Standing rule:** ship the simplest solution that works, and build it out of what already
+exists — a `.templates/` module, an existing cont-init script, the pattern a sibling add-on
+already uses for the same problem. 120+ add-ons are maintained by one person: a homogeneous repo
+where every add-on solves a problem the same way is worth more than a locally nicer bespoke
+design. Prefer reusing or extending over adding a parallel implementation, and when you must add
+something new, spell it the way the rest of the repo spells it (naming, option names, script
+numbering, file layout). Complexity is bought only by a **measurement** showing a concrete,
+user-visible cost on a real host — never by reasoning about hypothetical performance.
 
 **Repo layout.** `alexbelgium/hassio-addons`; each add-on is a top-level directory. This skill is
 checked in at `.claude/skills/hassio-addon-workflow/` (canonical copy). Set the skill root once,
@@ -73,7 +78,14 @@ before it costs a full analysis pass. Measurement methodology, gotchas, and real
 
 ## 3. Plan — choose the mechanism level, then Codex reviews it (full loop)
 
-Rank mechanisms, pick the lowest (simplest) one that solves it, and state the choice in the plan:
+Look for prior art first: grep `.templates/` and the other add-ons for something that already
+solves this (`grep -rl "<knob or pattern>" --exclude-dir=.git .` — search everything, not just
+`*.sh`: the mechanism may live in a `Dockerfile`'s `ARG MODULES=` or an extensionless s6 `run`
+file). If an add-on already handles it, the plan is "do what that one does" — say so, and say why
+the existing mechanism can't be reused if you're not reusing it.
+
+Then rank mechanisms, pick the lowest (simplest) one that solves it, and state the choice in the
+plan:
 
 1. A config value — an option, a schema constraint, an existing env var.
 2. An existing knob the base image already reads (`MAX_RES`, `DRINODE`, `SELKIES_*`).
@@ -110,7 +122,13 @@ not just the happy path.
 
 Before requesting review, check: did the diff stay at the ladder level chosen in step 3? Can this
 be solved by deleting instead of adding? Is the fix bigger than what it fixes? How does it fail in
-three years? Case studies of what happens when this check is skipped: `references/simplify.md`.
+three years? And on reuse: does any hunk reimplement something `.templates/`, another script in
+this add-on, or a sibling add-on already does — and if a future add-on hits this same problem,
+will it find one way to solve it or two? Fold a near-duplicate into the existing mechanism, or
+justify the divergence in the PR body — but never at the cost of an isolation rule
+`references/traps.md` documents: scripts shared by symlink with the webtop add-ons take a new
+numbered script, not an edit. Case studies of what happens when this check is skipped:
+`references/simplify.md`.
 
 ## 6. Codex attacks the code (full loop only)
 

.github/scripts/ai_triage_context.sh

@@ -77,7 +77,10 @@ if [ -n "$ADDON" ]; then
   {
     echo
     echo "## Addon files: ${ADDON}/"
-    if ! git sparse-checkout set --no-cone .github/prompts .github/scripts "$ADDON" 2>&1; then
+    # `set` REPLACES the checkout list, so .templates has to be repeated here
+    # or the workflow's sparse-checkout of it is silently undone at this point
+    # — which is exactly the state that starved #2949 of its turn budget.
+    if ! git sparse-checkout set --no-cone .github/prompts .github/scripts .templates "$ADDON" 2>&1; then
       # Swallowing this used to leave ADDON resolved with no files behind it,
       # so the classifier could still reach high confidence off the addon
       # name alone. Say so explicitly, in the same word Rule 2 already keys

.github/workflows/on_issues_ai_triage.yaml

@@ -41,6 +41,16 @@ on:
       issue:
         description: "Issue number to (re-)triage manually"
         required: true
+      source:
+        # Explicit provenance, set only by the catch-up job below. Previously
+        # this was inferred from github.actor, which is brittle: a re-run, a
+        # dispatch via a PAT or App, or another maintainer all change it, and
+        # the dangerous direction is the false negative — an automated retry
+        # that is never recognised as one keeps retrying forever. An input the
+        # scheduler sets explicitly cannot drift with GitHub's actor semantics.
+        description: "Set to 'catchup' by the daily catch-up job; leave blank for a manual re-triage"
+        required: false
+        default: ""
 
 permissions:
   contents: read
@@ -131,9 +141,17 @@ jobs:
         with:
           fetch-depth: 1
           persist-credentials: false
+          # .templates holds the shared build/runtime scripts (ha_entrypoint.sh,
+          # ha_automodules.sh, the cont-init modules) that nearly every add-on
+          # depends on, so a large share of reports can only be explained by
+          # reading them. Without it the classifier burned 6 of its turns on
+          # #2949 hunting for files that were not checked out, then died on
+          # max_turns. It is a small directory — cheaper to ship than to search
+          # for and not find.
           sparse-checkout: |
             .github/prompts
             .github/scripts
+            .templates
           sparse-checkout-cone-mode: false
 
       - name: Build context bundle
@@ -190,10 +208,43 @@ jobs:
           # actor=alexbelgium, a User.
           allowed_bots: "github-actions"
           show_full_output: true
+          # Stated up front, because a wrong guess about the environment costs
+          # turns the analysis then does not have. On #2949, under the earlier
+          # 12-turn budget, the model spent 3 turns retrying Bash and 6 hunting
+          # files outside the sparse checkout and died before reaching a
+          # verdict. The budget is 25 now, but it is meant to buy analysis, not
+          # more failed probing — keep this in step with --max-turns below.
           prompt: |
             Read /tmp/ai-triage/context.md, then follow the instructions in
             .github/prompts/issue-classify.md exactly.
 
+            Before you start, two facts about this environment. Both are hard
+            limits, not preferences — working around them is not possible and
+            costs you turns you need for the analysis.
+
+            You have exactly three tools: Read, Glob and Grep. There is no
+            Bash. Do not try to run `find`, `ls`, `cat` or any other command;
+            those calls fail and are not retryable. Use Glob where you would
+            have used `find`, and Grep where you would have used `grep`.
+
+            This is a SPARSE checkout of a 100+ add-on monorepo. Only these
+            paths exist on disk — everything else is absent, and searching for
+            it will find nothing no matter how you phrase the search:
+              * .templates/          shared build and runtime scripts that most
+                                     add-ons rely on (ha_entrypoint.sh,
+                                     ha_automodules.sh, the cont-init modules)
+              * .github/prompts/, .github/scripts/
+              * the single add-on directory named in the context bundle, if it
+                was resolved — the bundle says which, or says UNRESOLVED
+            Other add-ons are NOT present. If the bundle says UNRESOLVED, no
+            add-on source is on disk at all: judge from the bundle alone and
+            set confidence accordingly rather than searching for the code.
+
+            You have a budget of 25 turns. The context bundle already contains
+            the issue, its comments, the add-on's config/Dockerfile/docs, its
+            recent commits and candidate duplicates — so read it first and
+            spend turns only on what it does not already answer.
+
             Return your verdict as structured output. Do NOT comment on or
             label the issue yourself.
           # The model gets NO write capability of any kind — not Bash, not
@@ -214,7 +265,7 @@ jobs:
           claude_args: |
             --model claude-sonnet-5
             --effort low
-            --max-turns 12
+            --max-turns 25
             --allowedTools "Read,Glob,Grep"
             --json-schema '{"type":"object","properties":{"verdict":{"type":"string","enum":["owned","duplicate","needs-info","question","upstream-bug","addon-bug","feature-request"]},"addon":{"type":"string"},"confidence":{"type":"string","enum":["high","medium","low"]},"duplicate_of":{"type":"integer"},"labels":{"type":"array","items":{"type":"string"},"maxItems":2},"root_cause_hint":{"type":"string"},"comment":{"type":"string"}},"required":["verdict","confidence"]}'
 
@@ -225,11 +276,18 @@ jobs:
           ISSUE: ${{ github.event.issue.number || inputs.issue }}
           REPO: ${{ github.repository }}
           EVENT_NAME: ${{ github.event_name }}
+          # Distinguishes the automated catch-up retry from a manual
+          # re-triage — see is_automated_retry below.
+          DISPATCH_SOURCE: ${{ inputs.source }}
           CLASSIFY_OUTCOME: ${{ steps.classify.outcome }}
           # Through env, never interpolated into the script body: this string
           # is model output and "${{ }}" inline would splice it into the shell
           # source itself.
           STRUCTURED: ${{ steps.classify.outputs.structured_output }}
+          # Written by the action even when it fails (setExecutionFileOutputIfPresent
+          # runs in its catch block), which is what lets the max-turns check below
+          # work on exactly the runs that need it.
+          EXECUTION_FILE: ${{ steps.classify.outputs.execution_file }}
         run: |
           set -euo pipefail
           mkdir -p /tmp/ai-triage
@@ -243,6 +301,67 @@ jobs:
             gh issue edit "$ISSUE" --repo "$REPO" --add-label ai:needs-info >/dev/null 2>&1 || true
           }
 
+          # Is this the automated second look, rather than a first attempt?
+          # EVENT_NAME alone is not enough: workflow_dispatch is BOTH the daily
+          # catch-up retry and the maintainer's manual re-triage, so keying on
+          # it alone escalates a hand-dispatched first attempt immediately.
+          # The catch-up therefore states its provenance explicitly via the
+          # `source` input. Inferring it from github.actor instead was rejected:
+          # a re-run, a PAT- or App-issued dispatch, or a different maintainer
+          # all change the actor, and the failure that matters is the false
+          # NEGATIVE — an automated retry not recognised as one would never
+          # escalate and would retry that issue forever.
+          # Unknown provenance is treated as "not the automated retry", which
+          # is safe here because every non-escalating max-turns path below ends
+          # in a red run rather than a silent green one.
+          is_automated_retry() {
+            [ "${EVENT_NAME:-}" = "workflow_dispatch" ] && [ "${DISPATCH_SOURCE:-}" = "catchup" ]
+          }
+
+          # Hand the issue to a human and take it out of the retry rotation.
+          # Returns non-zero if the labels did not actually land — callers must
+          # treat that as a failure rather than reporting a hand-off that never
+          # happened, which would leave the issue unlabelled and back in the
+          # retry rotation it was supposed to leave.
+          escalate_to_human() {
+            # Best effort: the label usually exists, and `gh issue edit` fails
+            # on its own below if it does not.
+            gh label create ai:needs-human --repo "$REPO" --color ededed >/dev/null 2>&1 || true
+            # NOT suppressed with `|| true`. ai-triage and ai:needs-info come
+            # off in the same call: leaving ai-triage would keep an issue we
+            # just escalated sitting in tier 2's unattended queue, and leaving
+            # ai:needs-info would let a reporter reply silently re-trigger
+            # classification behind the human's back. Removing a label the
+            # issue does not carry is a no-op, so this cannot fail spuriously.
+            gh issue edit "$ISSUE" --repo "$REPO" \
+              --add-label ai:needs-human \
+              --remove-label ai-triage --remove-label ai:needs-info >/dev/null 2>&1
+          }
+
+          # Did the run die on its turn budget rather than on a workflow fault?
+          # The execution file is a JSON array of SDK messages; the terminal
+          # result object carries subtype "error_max_turns".
+          #
+          # This MUST fail closed: a false positive here downgrades a genuine
+          # workflow failure from a red run to a warning, which is the exact
+          # silent-failure class this workflow was rebuilt to remove. Hence the
+          # explicit `type == "array"` root check — without it `.[]?` happily
+          # iterates the VALUES of an object, so if the action ever changed the
+          # file's shape, {"result":{"subtype":"error_max_turns"}} would match
+          # and mask the failure. Anything that is not the array we expect is
+          # treated as "not max turns" and falls through to the loud path.
+          # The `?` and per-element type check keep a non-object element from
+          # aborting the step under set -e.
+          hit_max_turns() {
+            [ -n "${EXECUTION_FILE:-}" ] && [ -s "${EXECUTION_FILE:-}" ] || return 1
+            jq -e '(type == "array") and
+                   any(.[]?;
+                       (type == "object") and
+                       (((.subtype? // "") == "error_max_turns") or
+                        ((.terminal_reason? // "") == "max_turns")))' \
+               "$EXECUTION_FILE" >/dev/null 2>&1
+          }
+
           # GATE 1 — did the action itself run? This is checked BEFORE looking
           # at the payload, because the action can fail *after* having written
           # a valid structured output: the object would sail through the shape
@@ -260,6 +379,38 @@ jobs:
           # explicit exit 1 the job would report success.
           if [ "${CLASSIFY_OUTCOME:-}" = "failure" ]; then
             restore_needs_info
+
+            # ...with one exception. Exhausting the turn budget is NOT a
+            # workflow fault: the action ran fine and this particular issue was
+            # just too tangled to finish inside the turn budget. Treating it as systemic
+            # meant #2949 failed red and stayed unlabelled, so the catch-up
+            # re-dispatched it every day forever — and being the newest issue
+            # it took the first of only five daily slots each time.
+            # So it is handled like GATE 2 below instead: one retry, then a
+            # human. Warning rather than error, because a red run per day for a
+            # per-issue condition is alarm fatigue, and the outcome is recorded
+            # durably on the issue itself rather than only in a run log.
+            # A green run is only ever justified once the outcome is recorded
+            # somewhere durable. On the automated second look that is the
+            # ai:needs-human label, and only if it actually landed. On a first
+            # attempt nothing is recorded anywhere but this annotation, so
+            # exiting 0 there would be precisely the "green run, work silently
+            # dead" state that left triage broken for weeks. It costs at most
+            # one red run per problem issue, not one per day, because the
+            # second look ends the retry rotation either way.
+            if hit_max_turns; then
+              if is_automated_retry; then
+                echo "::warning::second attempt for #$ISSUE also ran out of turns, handing it to a human"
+                if ! escalate_to_human; then
+                  echo "::error::could not label #$ISSUE ai:needs-human — it is NOT escalated and stays in the retry rotation"
+                  exit 1
+                fi
+                exit 0
+              fi
+              echo "::error::classification for #$ISSUE ran out of turns; leaving it for the catch-up to retry once, after which it goes to a human"
+              exit 1
+            fi
+
             echo "::error::the Classify action failed for #$ISSUE — this is usually a workflow-level fault affecting every issue, so the issue is left untouched for a retry. See the Classify step."
             exit 1
           fi
@@ -276,29 +427,23 @@ jobs:
           #
           # Reaching here means the failure is specific to THIS issue — the
           # model looked at it and produced nothing usable — so a retry is
-          # worth exactly one attempt. A workflow_dispatch is the catch-up or
-          # a manual re-triage, i.e. the second look, so hand it to a human
-          # rather than re-dispatching the same issue every day forever;
-          # ai:needs-human is in the catch-up exclusion search, so it drops out
-          # of the queue instead of starving newer issues behind it.
+          # worth exactly one attempt. Only a dispatch carrying source=catchup
+          # counts as that second attempt (is_automated_retry above); a manual
+          # workflow_dispatch is a first look and does NOT escalate, leaving
+          # the issue unlabelled so the catch-up still gets its own go. On the
+          # automated retry, hand it to a human rather than re-dispatching the
+          # same issue every day forever; ai:needs-human is in the catch-up
+          # exclusion search, so it drops out of the queue instead of starving
+          # newer issues behind it.
           if [ ! -s "$F" ] || ! jq -e 'type == "object"' "$F" >/dev/null 2>&1; then
             restore_needs_info
             echo "::warning::no usable verdict produced for #$ISSUE"
-            if [ "${EVENT_NAME:-}" = "workflow_dispatch" ]; then
+            if is_automated_retry; then
               echo "::warning::second attempt produced no verdict, handing #$ISSUE to a human"
-              gh label create ai:needs-human --repo "$REPO" --color ededed >/dev/null 2>&1 || true
-              # Drop the retry triggers in the same call. The catch-up search
-              # already excludes both, so this only bites on a MANUAL
-              # re-triage of an issue that still carries them — but there it
-              # matters: leaving ai-triage would keep an issue we just handed
-              # to a human sitting in tier 2's unattended fix queue, and
-              # leaving ai:needs-info would let a reporter reply silently
-              # re-trigger classification behind the human's back. The normal
-              # verdict path below already clears stale control labels; this
-              # keeps the escalation path consistent with it.
-              gh issue edit "$ISSUE" --repo "$REPO" \
-                --add-label ai:needs-human \
-                --remove-label ai-triage --remove-label ai:needs-info >/dev/null 2>&1 || true
+              if ! escalate_to_human; then
+                echo "::error::could not label #$ISSUE ai:needs-human — it is NOT escalated and stays in the retry rotation"
+                exit 1
+              fi
             fi
             exit 0
           fi
@@ -484,7 +629,7 @@ jobs:
           while IFS= read -r n; do
             [ -n "$n" ] || continue
             echo "re-dispatching tier 1 for #$n"
-            gh workflow run "AI issue triage" --repo "$REPO" -f issue="$n" || {
+            gh workflow run "AI issue triage" --repo "$REPO" -f issue="$n" -f source=catchup || {
               echo "::error::could not dispatch classify for #$n"
               FAILED=$((FAILED + 1))
             }