mirror of https://github.com/alexbelgium/hassio-addons.git synced 2026-08-23 13:23:32 +02:00

Files

Alexandre 3e561a3f9f refactor(skill): shorten hassio-addon-workflow via progressive disclosure (#2942 )

* refactor(skill): shorten hassio-addon-workflow via progressive disclosure

SKILL.md was 365 lines, loaded in full on every add-on task. Split it per
Anthropic's Agent Skills best practices: keep steps, completion criteria,
and the mechanism ladder inline; push rationale, war-story examples, and
Codex CLI invocation details into reference files loaded only when that
branch is taken.

- SKILL.md: 365 -> 171 lines. The "ship the simplest solution" rule was
  stated three times; now once. Steps 3/6/9 keep their load-bearing
  checklist but point to detail files instead of inlining it.
- references/evidence.md (new): measurement methodology, the
  host-generalization failure examples, the merged-and-inert case studies
- references/codex-review.md (new): CLI invocation, prompt guidance,
  plan-attack checklist
- references/simplify.md (new): mechanism-ladder case studies

Also replaces the old "Token efficiency" section (which named this
maintainer's personal MCP tools - rtk/headroom/tokensave - not guaranteed
present for CI agents or other collaborators using the checked-in copy)
with a subagent-delegation instruction: Codex's plan/code review and
PR-comment triage on >5 threads should run in a subagent that returns a
condensed summary, not raw output, into the calling session.

* fix(skill): address PR review feedback

- SKILL.md: define $SKILL once and state that all scripts/ and references/
  shorthand paths are relative to it — they read as repo-root-relative
  otherwise and don't resolve from an add-on directory
- codex-review.md: drop the reference to a "global CLAUDE.md note" that
  isn't in this repo's CLAUDE.md; describe --sandbox read-only accurately
  (reads allowed, writes/exec blocked, approvals disabled) instead of
  "cannot run anything"; add the missing & so the example is actually
  backgrounded as the prose claims
- evidence.md: use smaps_rollup for process RSS — plain smaps prints one
  Rss: line per mapping (47 for a trivial process here), not a total
- simplify.md: drop the absolute "removals cannot regress" claim, which
  contradicted the /dev/shm case study in evidence.md

* refactor(skill): apply independent quality-review findings

From an independent model review of the skill against the Agent Skills
best-practice guides:

- traps.md: CHANGELOG was described as "the only hard gate", contradicting
  SKILL.md — the HA add-on linter and the image build also block PRs
  (verified against onpr_check-pr.yaml). Gates list corrected.
- codex-review.md: the "attack your own plan" checklist is run by the main
  agent, but lived in the file whose stated consumer is the delegated
  subagent — an agent that delegates correctly would never see it. Moved
  inline into SKILL.md step 3.
- SKILL.md: delegation instruction now says exactly what to do (spawn a
  subagent whose prompt includes references/codex-review.md and follows its
  invocation) instead of "delegated per the rule above".
- SKILL.md: traps.md is no longer mandatory reading on the light path —
  the light-path facts it holds (versioning, CHANGELOG format) are inline
  in step 7; it stays required when touching scripts/Dockerfiles/env.
- SKILL.md: dropped the bundled-files table (every row already cited at
  point of use) and the "read the script when you use it" anti-instruction
  — scripts are run, not read. 177 -> 167 lines.
- description: 982 -> 550 chars; removed workflow narrative that does
  nothing for skill selection and the stale-prone model name, kept all
  trigger terms.

* fix(skill): note that CI hard gates skip on non-addon PRs

The three hard gates in onpr_check-pr.yaml (changelog check, addon-linter,
check-build) are each matrixed over check-addon-changes.outputs.changedAddons
and if:-skipped when it is '[]'. A PR touching only docs, .github/ or .claude/
therefore shows them as "skipping" rather than passing — which should not be
read as a green build. Verified against onpr_check-pr.yaml lines 64, 83, 101
and against this PR's own check output.

2026-08-05 10:11:11 +02:00

3.2 KiB

Raw Permalink Blame History

Evidence — measurement methodology and case studies

Why summed RSS and reserved-vs-resident both matter

Summed RSS double-counts shared pages. Removing a duplicate process frees its private memory, not its RSS. scripts/measure.sh reports PSS and private alongside RSS — quote private when arguing "removing this saves N MB".
A big mapping is not necessarily resident. Large SysV/tmpfs segments are lazily populated; reserved size is reported separately from resident for this reason.
/proc/meminfo and free show host figures (no memory cgroup namespace here) — never attribute those to the add-on.
Sample duration matters: a 3 s CPU sample measured 2.3% where a 20 s sample measured 21.6% for the same process. Use ≥20 s for anything you report.

Before asserting anything, ask what would show it false

"This process is duplicated" → is it? ps -ef --forest, compare parents and start times.
"This costs 500 MB" → is it resident? grep Rss /proc/<pid>/smaps_rollup — plain smaps prints one Rss: line per mapping (dozens of them), not a process total.
"This block never runs" → is its payload in the image? command -v, apt history.
"The flag isn't set" → tr '\0' '\n' < /proc/<pid>/cmdline.

When you correct yourself mid-analysis, keep the correction visible in your notes and in what you report — a retracted claim that stays retracted is worth more than one quietly dropped.

The failure mode this loop keeps producing

Every bug shipped from the source session came from one move: measuring this host correctly, then generalising it to all hosts.

/dev/shm was 7.7 GB here, so a flag looked useless — but Home Assistant ignores shm_size, so elsewhere it is Docker's 64 MB default and removing the flag reintroduces a crash loop.
An MCP entry was identified by its URL — but that URL is the documented default, so the rule would have deleted a user's hand-written configuration.
A GPU probe created a hardware context — but that proved the driver worked, not that Chromium's GPU path did.

The pattern is always inference standing in for detection. Before changing a default, ask what this is like on a host unlike yours. Prefer detecting the condition at runtime over asserting it. When ownership matters, record it rather than infer it.

"Merged and inert" — CI passing proves the build works, not that the change does anything

Both changes in the session that produced this skill passed CI, merged, and were inert:

The Xvfb resolution cap wrote its env file correctly and Xvfb still started at the base-image default — wrong env mechanism for that service.
The GPU flags reached Chromium's command line exactly as intended, and the GPU process still reported --use-gl=disabled, having overridden them after its own init failed.

Once the rebuilt add-on is running, re-run the measurement that motivated the work. Some fixes cannot be self-verified — a service that reads its environment only at start makes an env-var fix unproven until the add-on restarts, which needs the user or ha-cli with their agreement. If you cannot restart, the change is Assumed, not Verified, and must be reported that way.

3.2 KiB Raw Permalink Blame History